Linux-2.5.28

The most fundamental part of this has already been discussed a lot and posted to the kernel list, including a lot of fixes (all hopefully integrated). That's obviously the removal of the global irq lock. In the short term (famous last words) that breaks a number of SMP configurations, but fixing them should not be horribly hard.

A lot of other stuff here too - the regular USB updates, fbdev updates, m68k and ppc64 updates, IDE fix, and a sync-up with Al. Serial lawyer all shook up (the irq lock kind of forced that one, but it's certainly been pending long enough..)

Go wild,

Linus

---

Summary of changes from v2.5.27 to v2.5.28
============================================

<[email protected]>:
o IDE-101

<[email protected]>:
o drivers/usb/misc/tiglusb.c v1.04

<[email protected]>:
o Added help for the Toshbia and Permedia3 framebuffer devices. Small fixes for the ATI 128 card and the logo drawing code in fbcon.c. Proper handling of data for pci handling
o Added VBI support to VESA. ATY 128 compiles now :-)
o Removed all old fbgen code. Small cleanups
o Synced up to m68k changes

<[email protected]>:
o consolidate task->mm code + fix

<[email protected]>:
o USB: rtl8150 updated

Alexander Viro <[email protected]>:
o make hfs use regular semaphores
o Use wipe_partitions() where appropriate
o partition parsing cleanup
o block device size cleanups
o partition handling locking cleanups
o blk_ioctl() not exported anymore
o paride cleanup and fixes
o SCSI ->bios_param() switched to struct block_device *
o removal of dead prototypes
o jffs kdev_t cleanups
o fix for nfs_unlink and vfs_unlink
o Fix dcache deadlock introduced by previous fix

Andrew Morton <[email protected]>:
o disable highpte in rmap kernels
o page-writeback.c compile warning fix

Anton Blanchard <[email protected]>:
o ppc64: enable eeh on non-LPAR
o ppc64: copy_user_page and clear_user_page now take a page * ppc64:
updates for mmu gather code
o ppc64: Use non context synchronising mtmsrd. Cleanup init.c
o ppc64: 64 and 32 bit signal cleanups from Stephen Rothwell
o ppc64: Implement copy_siginfo_to_user32 from Stephen Rothwell
o ppc64: Remove POWER4 special case for cache_decay_ticks
o ppc64: Initial DISCONTIGMEM and NUMA support
o ppc64: Only use irq balancing on openpic for the moment
o ppc64: update ppc64 tlb batch code
o ppc64: Add fls
o ppc64: POWER4 lazy icache flushing
o ppc64: add missing gcc barrier in softirq code
o ppc64: export timebase frequency in /proc/cpuinfo
o pSeries HVC console: Fix hang up race - from Dave Engebretsen
o pSeries HVC console: Add SYSRQ and handle errors better from Dave Engebretsen
o pSeries firmware flash support from Todd Inglett
o ppc64: iSeries updates
o ppc64: increase IRQ_KMALLOC_ENTRIES
o ppc64: ptrace cleanup from Stephen Rothwell
o ppc64: ptrace32 fix when tracing 64bit tasks from Will Schmidt
o ppc64: config.h resync and remove some stale code ppc64: turn off STRICT_MM_TYPECHECKS
o pSeries HVC console: fix hvc_hangup definition
o ppc64: remove __openfirmware and __chrp
o ppc64: include/asm/md.h is not used any more
o ppc64: remove some stale code
o ppc64: updates for 2.5.21
o ppc64: Makefile updates
o ppc64: non linear cpu support
o ppc64: define new scheduler hooks
o ppc64: update for recent 32/64bit binutils
o ppc64: Fix warnings
o ppc64: Fix for 32 bit ELF timeval handling, from sparc64
o ppc64: define SMP_CACHE_BYTES and cleanup HZ handling
o ppc64: use symbolic names for fault types
o ppc64: _switch_to -> __switch_to
o ppc64: defconfig update
o ppc64: exception should be 0x480 for instruction SLB miss - jimix
o ppc64: misc cleanups
o ppc64: __clear_user should return number of bytes not copied
o ppc64: iSeries update - from 2.4
o ppc64: add comment and missing include
o ppc64: Add northstar CPU
o ppc64: Add winnipeg support
o ppc64: UP fix for irq affinity
o ppc64: add POWER4+ (GQ) support
o ppc64: add rmap.h
o ppc64: workaround for gcc 3.1, otherwise we busy loop in pte_chain_lock()
o ppc64: fix test_bit and remove workaround in cpu_relax
o ppc64: big IRQ lock removal
o ppc64: Fix for spurious interrupts in LPAR without ISA
o ppc64: merge some 2.4 fixes
o ppc64: missed during last merge
o ppc64: Designated initializers from Rusty
o ppc64: add Config.help
o ppc64: Optimise for 630 by default
o ppc64: put paca in r13 and fix non zero boot cpu
o flags must be unsigned long]
o Make tlb_remove_tlb_entry take ptep]
o Fix token ring compile]

Christopher Hoover <[email protected]>:
o for ohci on SA-1111
o set_device_description oops fixage mk2

Dave Kleikamp <[email protected]>:
o Clean up Documentation/filesystems/jfs.txt
o JFS: Use cond_resched()
o JFS: cosmetic syncup with 2.4 code
o JFS: Replace depreciated initializer syntax with C99 style

David Brownell <[email protected]>:
o usb_set_interface() doc
o hid_ff_init could not find initializer

Geert Uytterhoeven <[email protected]>:
o M68k update (1-49)

Greg Kroah-Hartman <[email protected]>:
o PCI Hotplug: fix i_nlink for root inode in pcihpfs
o PCI Hotplug: fix the dbg() macro to work properly on older versions of gcc
o USB pl2303: new device support added
o USB: rio500.c bugfix
o USB: usb-serial.h cleanups
o USB: changed the interface name to be a bit more unique

Hugh Dickins <[email protected]>:
o shm_destroy lock hang
o shmem_link duplicated test
o shmem_file_write double kunmap
o shmem_getpage_locked missing unlock

Ingo Molnar <[email protected]>:
o "big IRQ lock" removal, IRQ cleanups
o "big IRQ lock" removal docs
o Re: [patch] cli()/sti() cleanup
o irqlock patch 2.5.27-H6
o scheduler fixes

James Simmons <[email protected]>:
o Removal of nonexistant iplan16 support. Compile fix for aty128fb driver. Proper handling of PCI private data for fbdev drivers
o Removed old FB_COMPAT_XPMAC stuff. Ported over the Riva framebuffer driver over to the new api. Updated the Voodoo 1 driver
o Added Help info for Permedia 3 and Toshiba TX3912 graphics card support
o Updated Voodoo 1 documentation
o Finished the NVIDIA driver port to the new api. Killed a strtok in sstfb
o Added VBI support to VESA
o Supports more NVIDIA cards
o NVIDIA fixes to handle HSYNC and VSYNC flags. Set the registers to read the image data as big endian. Handle the different smem_len for different kinds of cards
o Cleanusp for the 3Dfx driver
o Finally touchs to the New mac framebuffer driver. Started the port of the ATI 128 driver to the new api. A few small optimizations and a bug fix for SUN 12x22 fonts with the new accel code
o M68K updates for there framebuffer devices
o More changes to port over teh ATI 128 driver to new api. Optimizations for fbgen and small bug fix for gen_update_var
o Ported over ATI 128 Rage driver to new api. A few config mistakes where fixed
o Ported SA1100 framebuffer over to new fbdev api
o Fixed bug for large logos. Also had to make a patch to handle X server reversing the image order programming verses how the riva fbdev driver does it
o Removed fbcon-vga.c and the old fbgen code. Ported over the SgiVW, OpenFirmware fbdev driver and started the Mach64 fbdev driver to the new api. A few simple typos as well
o Port step some changes at authors request
o Reversed so more changes
o Permiedia 2 support on PowerPC platform
o Updates to SIS framebuffer driver
o Porting Mach 64 drive over to new api

Jan Harkes <[email protected]>:
o uhci-hcd suspend fix

Jens Axboe <[email protected]>:
o add __blk_stop_queue() as locked variant of blk_stop_queue() and make cpqarray and cciss use these

[email protected] <[email protected]>:
o SCSI tape driver fixes for 2.5.27

Linus Torvalds <[email protected]>:
o Fix incoherent LDT at mmap exit
o Update ensoniq sound driver to new irq serialization
o Remove extraneous dget/dput pair in vfs_unlink() that confused the NFS client code wrt the exclusiveness of a dentry getting removed.
o Remove unused variable
o Fix up irqlock removal patch, avoid compiler warnings
o Fixups for previous changesets, avoid warnings etc

Neil Brown <[email protected]>:
o type safe(r) list_entry repacement: container_of
o MD - Fix two bugs that would cause sync_sbs to Oops
o MD - Convert struct initialised in md to "the new way"
o MD - Remove get_spare declaration and associated warning
o NFSD - new struct initialisers for nfsd

Richard Gooch <[email protected]>:
o Switched to ISO C structure field initialisers

Rik van Riel <[email protected]>:
o urgent rmap bugfix

Robert Love <[email protected]>:
o Re: "big IRQ lock" removal docs

Russell King <[email protected]>:
o Serial driver stuff
o [SERIAL] Rename files to remove serial_ prefix
o [SERIAL] Fix up various filenames, etc, from Ingo's merge of serial
o [SERIAL] Fix another SMP deadlock with modem status signal changes The original fix sent to Ingo for stop_tx didn't take account that the start_tx and stop_tx methods can be called from the device specific code under the port spinlock. Consequently, we move the spinlock to the callers of these methods. Documentation updated to reflect the change.
o [SERIAL] Fix deadlock in __uart_start introduced in previous cset Thanks to Zwane Mwaikambo for finding this.
o [SERIAL] Fix sa1100 serial driver stop function parameters

Rusty Russell <[email protected]>:
o AGP designated initializer update
o drivers/hotplug designated initializers

Trond Myklebust <[email protected]>:
o 2.5.27 fix potential spinlocking race

Error building 2.5.28:

ld -m elf_i386 -T arch/i386/vmlinux.lds -e stext
arch/i386/kernel/head.o arch/i386/kernel/init_task.o init/init.o --start-group arch/i386/kernel/kernel.o arch/i386/mm/mm.o kernel/kernel.o mm/mm.o fs/fs.o ipc/ipc.o security/built-in.o /usr/src/linux-2.5.28/arch/i386/lib/lib.a lib/lib.a /usr/src/linux-2.5.28/arch/i386/lib/lib.a drivers/built-in.o sound/sound.o arch/i386/pci/pci.o net/network.o --end-group -o vmlinux drivers/built-in.o: In function `put_cmd640_reg_pci1':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:223: undefined reference to save_flags' /usr/src/linux-2.5.28/drivers/ide/cmd640.c:224: undefined reference to cli'
drivers/built-in.o: In function `get_cmd640_reg_pci1':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:235: undefined reference to save_flags' /usr/src/linux-2.5.28/drivers/ide/cmd640.c:236: undefined reference to cli'
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:239: undefined reference to `restore_flags'
drivers/built-in.o: In function `put_cmd640_reg_pci2':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:249: undefined reference to save_flags' /usr/src/linux-2.5.28/drivers/ide/cmd640.c:250: undefined reference to cli'
drivers/built-in.o: In function `get_cmd640_reg_pci2':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:262: undefined reference to save_flags' /usr/src/linux-2.5.28/drivers/ide/cmd640.c:263: undefined reference to cli'
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:267: undefined reference to `restore_flags'
drivers/built-in.o: In function `put_cmd640_reg_vlb':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:277: undefined reference to save_flags' /usr/src/linux-2.5.28/drivers/ide/cmd640.c:278: undefined reference to cli'
drivers/built-in.o: In function `get_cmd640_reg_vlb':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:289: undefined reference to save_flags' /usr/src/linux-2.5.28/drivers/ide/cmd640.c:290: undefined reference to cli'
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:293: undefined reference to `restore_flags'
drivers/built-in.o: In function `put_cmd640_reg_pci1':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:227: undefined reference to `restore_flags'
drivers/built-in.o: In function `put_cmd640_reg_pci2':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:254: undefined reference to `restore_flags'
drivers/built-in.o: In function `put_cmd640_reg_vlb':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:281: undefined reference to `restore_flags'
drivers/built-in.o: In function `secondary_port_responding':
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:370: undefined reference to save_flags' /usr/src/linux-2.5.28/drivers/ide/cmd640.c:371: undefined reference to cli'
/usr/src/linux-2.5.28/drivers/ide/cmd640.c:379: undefined reference to restore_flags' /usr/src/linux-2.5.28/drivers/ide/cmd640.c:383: undefined reference to restore_flags'
make: *** [vmlinux] Error 1

Forgot to mention this is an SMP box. Without CONFIG_SMP it works fine.

-Paul Larson

This is known... there is a handful of drivers still using the old global IRQ methods that need a spring cleaning.

On UP, we "cheat" and define the global methods to the local ones and everything is happy. On SMP you are out of luck until the code if fixed.

Robert Love

Which drivers?

Cheers,
Bill

CMD640 for certain, but according to rml there are some others that need fixing as well.

-Paul Larson

But, on my UP:

if [ -r System.map ]; then /sbin/depmod -ae -F System.map 2.5.28; fi
depmod: *** Unresolved symbols in /lib/modules/2.5.28/kernel/net/irda/irda.o
depmod: cli
depmod: restore_flags
depmod: save_flags

(and no, CONFIG_SMP is not set :)

Ciao,

--alessandro

"my actions make me beautiful / and dignify the flesh"

                (R.E.M., "Falls to Climb")

He reported the cmd640 driver... but we really do not need 100 "it does not compile reports" on lkml. Just grep your tree for the old global IRQ methods... also, Ingo wrote a nice document in Documentation/ ..

Robert Love

It was not my intention to provide something unimportant to lkml, but the same config worked on the same machine in linux-2.5.27 and then was not working on 2.5.28. Since it had only recently broken, it's more likely that whatever broke it will still be fresh on somebody's mind and I was hoping that would make it easier for someone to fix.

Yes, a number of drivers won't compile on SMP because they want the global lock (that is gone). This includes the cmd640 IDE driver, and the parallel port driver (and a largish number of others too, but I think those two are the really common ones).

It should all work the same way it always did on UP, and hopefully the straggling drivers will be converted to use local spinlocks soon (and in some cases the global irq lock might not even be needed at all)

Linus

I'd just like to clear up some confusion here.

Under this cset is a change that adds '-g' to the top-level make flags. This cset, although it has my name and comments against it is actually a patch that added the serial drivers and extra bits that Ingo sent to Linus (and Linus kindly credited it back to me, and took the comments from one of my patch submissions for the serial drivers.)

The addition of '-g' is not, and has never been in my serial patches; please do not direct comments about the '-g' to me. Any problems relating to the serial changes should be forwarded to me.

Thanks.

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

In article <[email protected]>,

Oops, you're right. I'll undo that, and sorry for the attribution mess.

Linus

I had to export these 2 funcs.

@@ -2469,6 +2469,8 @@
 EXPORT_SYMBOL(uart_unregister_driver);
 EXPORT_SYMBOL(uart_register_port);
 EXPORT_SYMBOL(uart_unregister_port);
+EXPORT_SYMBOL(uart_add_one_port);
+EXPORT_SYMBOL(uart_remove_one_port);
 
 MODULE_DESCRIPTION("Serial driver core");
 MODULE_LICENSE("GPL");

Skip

Am Mit, 2002-07-24 um 23.13 schrieb Linus Torvalds:

What on earth is this??? I'm really surprised you accept this as a changelog entry especially when considering that there's no further information about the latest IDE changes on the mailinglist anymore...

Servus,
       Daniel

Actually, that patch _was_ on the mailing list, with lots of discussion.

And since none of the discussion was civil, it didn't get a changelog. But you can search it out yourself.

Linus

Am Don, 2002-07-25 um 00.52 schrieb Linus Torvalds:

So IDE-101 equals to the small snippet of code pasted somewhere in the evil flamewar?

Especially since the IDE code at the moment is not really something I would trust uncoditionally a bit more comentary would be adequate IMHO, even if it's just a: "Fixed race introduced in IDE-97, see flamewar"...

Perhaps not everyone wanting to use 2.5.x for some development is eager to disassemble a patch to see whether it might be usable or trash the partition even more badly (given that one has the knowledge to judge for her-/himself).

Servus,
       Daniel

Have you _looked_ at the full changelog? Apparently not.

The snippet was posted as part of the IDE-2.5.27 thread. Go look for it yourself. In addition, I asked Martin to send it to me separately, to verify that he hadn't had other issues too. The full changelog has that email.

Most of the IDE stuff is FUD and misinformation. I've run every single 2.5.x kernel on an IDE system ("penguin.transmeta.com" has everything on IDE), and the main reported 2.5.27 corruption was actually from my BK tree apparently due to the IRQ handling changes.

The thing I dislike is how people who apparently haven't even read the discussions, and didn't bother to look up the full changelog feel that they are perfectly fine to spread FUD and misinformation about the IDE layer.

Do we have issues there? Yes. But there are actually _more_ problems with people dissing the work than with the code itself.

Linus

Hi!

Yes, but the code actually is only this fix.

I wish it was misinformation.

No, I worked on this code really hard for some time. Just got tired listening to flacky ideas and syncing against intendation/whitespaces changes.

The biggest problem with IDE is not code or dissing people, but _the way_ of changes...

And the thing I dislike is how people who apparently haven't read carefully every ide-clean patch, and didn't bother to look up the full track of changes (2.4 -> 2.5) feel that they are perfectly fine to make such statements ;-).

Linus, please don't add me to .killmail for this mail yet :-).

It is really _the last_ mail on this subject.
Yep, I did too much of this dissing lately...
...and it was really not needed.

Regards
--
Bartlomiej

...need help getting a compile to complete. This problem exists with 2.5.27-28.
Here's the last lines from make...

ld -m elf_i386 -T arch/i386/vmlinux.lds -e stext arch/i386/kernel/head.o
arch/i386/kernel/init
_task.o init/init.o --start-group arch/i386/kernel/kernel.o arch/i386/mm/mm.o kernel/kernel.o mm
/mm.o fs/fs.o ipc/ipc.o security/built-in.o /usr/src/v2.5.28/arch/i386/lib/lib.a lib/lib.a /usr/ src/v2.5.28/arch/i386/lib/lib.a drivers/built-in.o sound/sound.o arch/i386/pci/pci.o net/network
.o --end-group -o vmlinux
drivers/built-in.o: In function `md_run_setup':
/usr/src/v2.5.28/drivers/md/md.c(.data+0xee34): undefined reference to `local symbols in discard
ed section .text.exit'
make: *** [vmlinux] Error 1

All the programs are better than specified in "Changes"...

    Gcc = 2.96-110 (RH7.3)
    binutils: ld = 2.11

thanks in advance, jeff

I need some ideas here.

I fixed the one mentioned below by changing md.c to a module. Then I2C had the same problem and fixed it by changing to a module. Now parport(?) has the problem and it's already a module. The current linker error is...

make[1]: Entering directory `/usr/src/v2.5.28/init'
Generating /usr/src/v2.5.28/include/linux/compile.h (updated)

gcc -Wp,-MD,./.version.o.d -D__KERNEL__ -I/usr/src/v2.5.28/include -Wall -Ws trict-prototypes -
Wno-trigraphs -O2 -g -fomit-frame-pointer -fno-strict-aliasing -fno-common - pipe -mpreferred-sta
ck-boundary=2 -march=athlon -nostdinc -iwithprefix
lude -DKBUILD_BASENAME=version -c -o
version.o version.c
ld -m elf_i386 -r -o init.o main.o version.o do_mounts.o
make[1]: Leaving directory `/usr/src/v2.5.28/init'
ld -m elf_i386 -T arch/i386/vmlinux.lds -e stext arch/i386/kernel/head.o
arch/i386/kernel/init
_task.o init/init.o --start-group arch/i386/kernel/kernel.o arch/i386/mm/mm.o kernel/kernel.o mm
/mm.o fs/fs.o ipc/ipc.o security/built-in.o /usr/src/v2.5.28/arch/i386/lib/lib.a lib/lib.a /usr/ src/v2.5.28/arch/i386/lib/lib.a drivers/built-in.o sound/sound.o arch/i386/pci/pci.o net/network
.o --end-group -o vmlinux
drivers/built-in.o: In function `parport_ieee1284_epp_write_data':
/usr/src/v2.5.28/include/asm/io.h:400: undefined reference to `local symbols in discarded sectio
n .text.exit'
make: *** [vmlinux] Error 1

----- Original Message -----
From: "jeff millar" <[email protected]>
To: "Kernel Mailing List" <[email protected]>
Sent: Wednesday, July 24, 2002 11:34 PM
Subject: Linux-2.5.28 link problem

ld fails to build vmlinux for all the recent v2.5 kernels. ld complains about

In function `parport_ieee1284_epp_write_data':
/usr/src/v2.5.29/include/asm/io.h:400:
undefined reference to `local symbols in discarded section .text.exit'

The machine is Redhat 7.3 on an Athlon, most of the recent Redhat updates. It's up to date with .../Documentation/Changes
I've tried make mrproper
I've tried switching between modules and compiled-in, modules get further
=========================================
$tail /usr/src/linux/make.out
make[1]: Leaving directory `/usr/src/v2.5.28/init'
ld -m elf_i386 -T arch/i386/vmlinux.lds -e stext arch/i386/kernel/head.o
arch/i386/kernel/init
_task.o init/init.o --start-group arch/i386/kernel/kernel.o arch/i386/mm/mm.o kernel/kernel.o mm
/mm.o fs/fs.o ipc/ipc.o security/built-in.o /usr/src/v2.5.28/arch/i386/lib/lib.a lib/lib.a /usr/ src/v2.5.28/arch/i386/lib/lib.a drivers/built-in.o sound/sound.o arch/i386/pci/pci.o net/network
.o --end-group -o vmlinux
drivers/built-in.o(.data+0x5174): undefined reference to `local symbols in discarded section .te
xt.exit'
make: *** [vmlinux] Error 1

Could you send your .config?

TIA
Adrian

You only think this is a free country. Like the US the UK spends a lot of
time explaining its a free country because its a police state.
								Alan Cox

Am Don, 2002-07-25 um 03.08 schrieb Linus Torvalds:

I was merely requesting a bit more verbose information in your regular changelog, the whole thing is quite exhaustive but this entry didn't really fit and contained no useful information at all.

I will definitely consider reading the "full changelog" although I cannot remember having read anything about such a thing before this thread.

Exactly what I said, no?

This is very encouraging information that had been missing from the threads at all: a success story from a person actually trusting und using this thing.

I for one did read the discussion(s) but it's really hard to map IDE-101 to some tiny patch in a huge tree of mails.

I appreciate Martins work and even more your word on it that it's pretty stable.

Keep on the good work and let us end this thread for good.

Servus,
       Daniel

Linus, Linus, how can you say something so naive?
I need not tell you that one user without problems does not imply that nobody will have problems.

A few people reported lost filesystems. Many more reported mild filesystem damage. And now you also report mild filesystem damage.

FUD? Fear? Yes, the fear is justified for whoever does not have backups. Uncertainty? Yes, when the filesystem is damaged again, it is not quite clear what causes it. Doubt? Yes, many people doubt whether they can afford to run 2.5.recent.

This evening I ran vanilla 2.5.29 and was rewarded with mild filesystem damage. 91 files in /lost+found. Nothing. A few kernel versions ago it was three orders of magnitude worse.

IDE? 2.4.17 and 2.5.27+Jens are stable for me in ordinary use. IRQ? Quite possible.
My third candidate is USB. Systems without USB are clearly more stable.

Andries

USB may have problems but on my test sets with 2.5.of those that booted, the scsi ones are pretty stable, the IDE ones eat disks or hang (mostly hang). USB loaded on some of the IDE boxes, the SCSI test boxes dont have USB.

I've not tried the forward port of the stable IDE code with the test loads. My SMP 2.5.27 test set on 2.5.27-ac1 (all the bits of which are in 2.5.29) with symbios scsi on a dual PPro has been running for 6 days.

That's not what I'm saying. I'm saying that there _are_ problems with IDE, but that the real problem with IDE is that some people aren't even willing to help despite the fact that we do have a maintainer that actually can work with people.

I realize that so many people are probably used to the fact that IDE maintainers do not take patches from the outside that people have kind of given up on even working on IDE, but it doesn't help to have people only be negative (and btw, I'm definitely not talking about you - you've been exceedingly _positive_ in that you're still willing to test and report on problems. I'm talking about people who don't even bother to do bug-reports, but only trash-talk the maintenance).

No, I've not reported lost filesystems. I'm reporting that _others_ reported filesystem damage that was _not_ related to the IDE patches at all, yet were instantly blamed on the IDE patches.

And THAT is part of the problem. I don't know why, but the IDE subsystem brings out the worst in people.

This is, btw, one reason why I hate mid layers. People blame them for everything, and fixing it for one setup breaks it for another.

Hmm.. I doubt that's your problem, but you might just want to pester Martin about your particular IDE setup and see if some light eventually goes off somewhere.

I have this memory that you're using PIO mode? Please do make full details available, reminding people which exact setups are broken..

Linus

On that note, let me mention the machines I personally am using IDE, and apparently do not see problems: a dual PII with "Intel Corp. 82371AB PIIX4 IDE", and a P4 with "SiS 5513 IDE (rev 208)".

Both setups in DMA mode, both setups have one disk per channel (first channel is disk, second channel is CD-ROM).

So what are the patterns for "working" vs "broken"?

Linus

In the probably-not-useful department because I haven't tested on 2.5, my experience over a quite some time has been that you find a lot more problems when you are actively beating on both channels. There is some chipset, I suspect you know which but Andre certainly does, that is just basically busted when you use both channels. I've had so many problems with this that for any data I care about I plug in a 3ware controller and use that instead.

I have a diskscrubber program which runs the bits through a series of changes, it's pretty trivial to write but I can post mine if you like, it works for banging on the disk.

---
Larry McVoy            	 lm at bitmover.com           http://www.bitmover.com/lm 

You have too standard systems to see problems :-).

Unusual combinations or more quirky chipsets -> real problems.

Plus there are PIO problems (esp. multisector), but some of them (not all) are in 2.4 IDE forward port also.

Regards
--
Bartlomiej

The machine I usually try new kernels on is a 400 MHz Intel Pentium II.

% dmesg | grep hd
Kernel command line: auto BOOT_IMAGE=2.5.27axboe ro root=346 rootfstype=reiserfs hdc=ide-scsi
ide_setup: hdc=ide-scsi

    ide0: BM-DMA at 0xf000-0xf007, BIOS settings: hda:DMA, hdb:DMA
    ide1: BM-DMA at 0xf008-0xf00f, BIOS settings: hdc:DMA, hdd:DMA
    ide2: BM-DMA at 0x9c00-0x9c07, BIOS settings: hde:pio, hdf:pio
    ide3: BM-DMA at 0xa800-0xa807, BIOS settings: hdg:pio, hdh:pio

hda: Maxtor 91728D8, ATA DISK drive
hdb: Maxtor 91728D8, ATA DISK drive
hdc: Hewlett-Packard CD-Writer Plus 8200, ATAPI CD/DVD-ROM drive
hdd: CD-ROM 40X/AKU, ATAPI CD/DVD-ROM drive
hde: Maxtor 93652U8, ATA DISK drive
hdf: Maxtor 96147H6, ATA DISK drive
hda: host protected area => 1
hda: 33750864 sectors (17280 MB) w/512KiB Cache, CHS=2100/255/63
hdb: host protected area => 1
hdb: 33750864 sectors (17280 MB) w/512KiB Cache, CHS=2100/255/63
hde: host protected area => 1
hde: 71346240 sectors (36529 MB) w/2048KiB Cache, CHS=70780/16/63
hdf: host protected area => 1
hdf: 120064896 sectors (61473 MB) w/2048KiB Cache, CHS=119112/16/63
hdd: ATAPI 48X CD-ROM drive, 128kB Cache
hda: hda1 < hda5 hda6 hda7 > hda4
hda4: <unixware: hda8 hda9 hda10 hda11 hda12 hda13 hda14 >
hdb: hdb1 hdb2 hdb3 < hdb5 hdb6 hdb7 >
hde: hde1 hde2 hde3 < hde5 > hde4
hde2: <bsd: hde6 hde7 hde8 hde9 >
hdf: hdf1 hdf2 hdf3
...

Here hde and hdf live on a HPT366 card.

% dmesg | grep HPT
HPT366: IDE controller on PCI bus 00 dev 48
HPT366: detected chipset, but driver not compiled in!
HPT366: chipset revision 1
HPT366: not 100% native mode: will probe irqs later
HPT366: IDE controller on PCI bus 00 dev 49
HPT366: chipset revision 1
HPT366: not 100% native mode: will probe irqs later

[This is from dmesg on a 2.5.27+2.4ide.]

hdc is a CD writer (on ide-scsi)
hdd is a CDROM

No hdparm is used - the IDE is left as the kernel sets it.

I have seen (at least) two kinds of problems:
kernel hang and filesystem corruption.
The hang was always on hde. The corruption was mostly on hdb.

1) Hangs are caused by this HPT366 card. Early 2.5 kernels would not boot because they would hang as soon as hde was touched. The same happens for example with the SuSE 8.0 install kernel. Other kernels would boot but would hang when there was significant activity on hde or hdf.

2) A different type of problem would be that the superblock of the root filesystem (on hdb) was zeroed. I have seen this at least three times - no damage at all, except for a wiped superblock. Easily repaired with e2fsck -b N.

A less pleasant version of this is a wiped block different from the superblock, or a block in which all data has been shifted by a few bytes. On such an occasion e2fsck went totally berserk and after believing this one block decided that most of my filesystem was broken, and "repaired" it out of existence. (That was a filesystem different from the root filesystem.)

Also yesterday the damage was to a single block, this time to a reiserfs root filesystem. Lots of messages

is_leaf: free space seems wrong: level=1, nr_items=29, free_space=64 rdkey
vs-5150: search_by_key: invalid format found in block 8274. Fsck?
vs-13070: reiserfs_read_locked_inode: i/o failure occurred trying to find stat data of [328 634 0x0 SD]

all involving block 8274.

Turns out that one needs to boot from other media in order to repair a reiserfs root filesystem - it does not suffice to mount it read-only. So, maybe ext2 is more convenient than reiserfs on root.

It is not impossible that this corruption-type problem is created at reboot time.

Andries

Hm, then that would imply that all of my systems are unstable :)

Seriously, I don't know of any outstanding 2.5 USB issues that cause oopses right now, or effect stability. Any problems that people are having, they sure are not telling me, or the other USB developers about...

thanks,

greg k-h

Please please learn how to use __FUNCTION__ properly. I see the same crap over and over again in security. OK? Please tell me a way how to dual boot a system with the new host controller names between 2.4 and
2.5. Putting redundant alias lines in /etc/modules.conf didn't work.

{sigh} This has _nothing_ to do with the stability of the code :)

I'd be glad to fix this, if someone sends me a patch. I recently accepted just such a patch for my 2.4 USB tree, as it is something that eventually needs to get done.

And if you do send me such a patch, please test it out on older compiler versions. I just finally got the pci hotplug code fixed up after you sent in a patch "fixing" this issue.

Works for me :)

My modules.conf has the following two lines in it:
alias usb-controller usb-uhci alias usb-controller uhci-hcd

Works just wonderfully for 2.2, 2.4, and 2.5.

thanks,

greg k-h

"USB mass storage" in general.

hcd-pci.c: uhci-hcd @ 00:07.2, Intel Corp. 82371AB PIIX4 USB

I don't think you need to search in this kind of direction. The usb-storage code is just not very solid. It works more or less, but it is really easy to provoke an oops.

Since there has been so much mail this evening, let me provoke an oops just to show. Steps:

1. compile vanilla 2.5.29 with all built-in (also usb), except for usb-storage.

2a. boot it, do nothing, reboot - all is fine
2b. boot it, insmod usb-storage, rmmod usb-storage, reboot - all is fine 2c. boot it, connect four CF/SM card readers to a hub. Mount usbdevfs.
Look at them with usbview. Now insmod usb-storage. This generates some kernel messages about the probing, then silence. Wait for two minutes. Nothing. Still no prompt showing the completion of the insmod. Remove the four Smart Media card readers from the hub. No reaction. Ctrl-Alt-Del initiates a reboot, but the reboot hangs. Wait for a while. Nothing. Touch the (non-USB) keyboard. Oops.

This was a funny oops, rather different from those I usually see. The stack trace was:

put_queue < handle_scancode < handle_kbd_event < update_wall_time < timer_bh < keyboard_interrupt < handle_IRQ_event < do_IRQ < default_idle < default_idle < common_interrupt < default_idle < default_idle < default_idle < cpu_idle < rest_init.

Andries

[that was all for today, I am afraid - have no time to do Linux work today]

I reported an oops at shutdown and provided the trivial fix. It is the the standard kernel since 2.5.26, I think.

But there are still other oopses at shutdown for 2.5.27.

For 2.5.29 I reported
"> I booted 2.5.29 earlier this evening and was greeted by

Further improvement of usb-storage is possible.

Andries

That patch should be in the latest kernel, thanks. Let me know if you are still having that problem in .29

Oh yeah, I'm not saying that this is not true at all :) Matt added a BUG_ON() that seems to be hitting a lot of people, but I guess that was his intention.

thanks,

greg k-h

Yes, since 2.5.26.

No, I fixed that problem. But, as I told you:

[Maybe I forgot to tell you; I am a mathematician; tend to be fairly precise; thus, the above precisely describes the state of my knowledge yesterday evening: in the category "USB-induced oopses at reboot" one bug was fixed in 2.5.26; there are further such bugs still present in 2.5.27; concerning 2.5.29, it does not get far enough to decide: the usb-storage module hangs initializing.]

Andries

(Concerning this BUG_ON in transport.c: it should be commented out for the moment. First of all, nothing is wrong, I think, and secondly, we know already with certainty that it will happen, so nothing is learnt. Matt simultaneously came with some SCSI patches. More or less reasonable, although discussion was possible. But these were not applied in 2.5.29. If and when these or similar patches have been applied to the SCSI code one may consider enabling this BUG_ON again.)
(I have not yet looked at the BUG at scsiglue.c: 150.)

You need to look at the full changelog to see the full entry: see, for
example: http://lwn.net/Articles/5577/. Or, to save the wear on your web browser:

<[email protected]>
[PATCH] IDE-101

Here is a quick fix. I would like to synchronize with the irq handler changes as well. Becouse right now I know that preemption is killing the disk subsystem when moving data between disks using different request queues... In esp. It get's me in to do_request() with a queue in unplugged state. (Not everything is my fault, after all :-).

This does still leave open the question of why these patches no longer go to linux-kernel, though...

jon

Jonathan Corbet
Executive editor, LWN.net
[email protected]

           ^^^^^^^^^

must be a typo, it would be a bug to enter do_request() with the queue in a _plugged_ state, not vice versa.

Jens Axboe

Yes you are right. Anyway the described problem is indeed observable.

              ^^^^^^^

Shake `em harder, lets see what falls out. 8-)

Dave.

| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Yeah yeah, rub it in.

Linus "dyslexic" Torvalds

Run the serial port too fast and it won't crash... it'll sue you, american style :)

Alexander Hoogerhuis                               | [email protected]
CCNP - CCDP - MCNE - CCSE                          | +47 908 21 485
"You have zero privacy anyway. Get over it."  --Scott McNealy

Here's a small patch to get the i810_audio.c driver working again.

thanks,

greg k-h

diff -Nru a/sound/oss/i810_audio.c b/sound/oss/i810_audio.c
@@ -1733,7 +1733,7 @@
 		}
 
 		spin_unlock_irqrestore(&state->card->lock, flags);
-		synchronize_irq();
+		synchronize_irq(state->card->irq);
 		dmabuf->ready = 0;
 		dmabuf->swptr = dmabuf->hwptr = 0;
 		dmabuf->count = dmabuf->total_bytes = 0;

@@ -2814,15 +2814,15 @@
 		}
 		dmabuf->count = dmabuf->dmasize;
 		outb(31,card->iobase+dmabuf->write_channel->port+OFF_LVI);
-		save_flags(flags);
-		cli();
+		local_irq_save(flags);
+		local_irq_disable();
 		start_dac(state);
 		offset = i810_get_dma_addr(state, 0);
 		mdelay(50);
 		new_offset = i810_get_dma_addr(state, 0);
 		stop_dac(state);
 		outb(2,card->iobase+dmabuf->write_channel->port+OFF_CR);
-		restore_flags(flags);
+		local_irq_restore(flags);
 		i = new_offset - offset;
 #ifdef DEBUG
 		printk("i810_audio: %d bytes in 50 milliseconds\n", i);

Hmm..

	local_irq_save(flags);

Ah, sorry, I didn't get that from cli-sti-removal.txt. Actually it looks like cli-sti-removal.txt is a bit wrong, as there is no local_irq_save_off() function. I'll send a patch for that next.

This is _only_ a guess, as I do not know this hardware or driver at all, but this function is only called at device init time (from the PCI probe function), so I don't think anything else is going on in the driver at the same time to warrent SMP locking. It also looks like this is done to determine the "clocking" of the device, so interrupts need to be off for the CPU doing the determination.

But again, this is only a guess from glancing at the code for a very short time, and I should probably start using the ALSA version of this driver anyway :)

Here's a patch with the extra local_irq_disable() call removed.

thanks,

greg k-h

diff -Nru a/sound/oss/i810_audio.c b/sound/oss/i810_audio.c
@@ -1733,7 +1733,7 @@
 		}
 
 		spin_unlock_irqrestore(&state->card->lock, flags);
-		synchronize_irq();
+		synchronize_irq(state->card->irq);
 		dmabuf->ready = 0;
 		dmabuf->swptr = dmabuf->hwptr = 0;
 		dmabuf->count = dmabuf->total_bytes = 0;

@@ -2814,15 +2814,14 @@
 		}
 		dmabuf->count = dmabuf->dmasize;
 		outb(31,card->iobase+dmabuf->write_channel->port+OFF_LVI);
-		save_flags(flags);
-		cli();
+		local_irq_save(flags);
 		start_dac(state);
 		offset = i810_get_dma_addr(state, 0);
 		mdelay(50);
 		new_offset = i810_get_dma_addr(state, 0);
 		stop_dac(state);
 		outb(2,card->iobase+dmabuf->write_channel->port+OFF_CR);
-		restore_flags(flags);
+		local_irq_restore(flags);
 		i = new_offset - offset;
 #ifdef DEBUG
 		printk("i810_audio: %d bytes in 50 milliseconds\n", i);

Here's that patch.

thanks,

greg k-h

diff -Nru a/Documentation/cli-sti-removal.txt b/Documentation/cli-sti-removal.txt
@@ -94,10 +94,10 @@
 released.
 
 drivers that want to disable local interrupts (interrupts on the
-current CPU), can use the following five macros:
+current CPU), can use the following four macros:
 
   local_irq_disable(), local_irq_enable(), local_irq_save(flags),
-  local_irq_save_off(flags), local_irq_restore(flags)
+  local_irq_restore(flags)
 
 but beware, their meaning and semantics are much simpler, far from
 that of the old cli(), sti(), save_flags(flags) and restore_flags(flags)

@@ -107,11 +107,7 @@
 
     local_irq_enable()        => turn local IRQs on
 
-    local_irq_save(flags)     => save the current IRQ state into flags. The
-                                 state can be on or off. (on some
-                                 architectures there's even more bits in it.)
-
-    local_irq_save_off(flags) => save the current IRQ state into flags and
+    local_irq_save(flags)     => save the current IRQ state into flags and
                                  disable interrupts.
 
     local_irq_restore(flags)  => restore the IRQ state from flags.

Hi,

In my understanding things look rather like this:

@@ -96,8 +96,8 @@
 drivers that want to disable local interrupts (interrupts on the
 current CPU), can use the following five macros:
 
-  local_irq_disable(), local_irq_enable(), local_irq_save(flags),
-  local_irq_save_off(flags), local_irq_restore(flags)
+  local_irq_disable(), local_irq_enable(), local_save_flags(flags),
+  local_irq_save(flags), local_irq_restore(flags)
 
 but beware, their meaning and semantics are much simpler, far from
 that of the old cli(), sti(), save_flags(flags) and restore_flags(flags)

@@ -107,11 +107,11 @@
 
     local_irq_enable()        => turn local IRQs on
 
-    local_irq_save(flags)     => save the current IRQ state into flags. The
+    local_save_flags(flags)   => save the current IRQ state into flags. The
                                  state can be on or off. (on some
                                  architectures there's even more bits in it.)
 
-    local_irq_save_off(flags) => save the current IRQ state into flags and
+    local_irq_save(flags)     => save the current IRQ state into flags and
                                  disable interrupts.
 
     local_irq_restore(flags)  => restore the IRQ state from flags.

Regards, Thunder

(Use http://www.ebb.org/ungeek if you can't decode)
------BEGIN GEEK CODE BLOCK------
Version: 3.12
GCS/E/G/S/AT d- s++:-- a? C++$ ULAVHI++++$ P++$ L++++(+++++)$ E W-$
N--- o?  K? w-- O- M V$ PS+ PE- Y- PGP+ t+ 5+ X+ R- !tv b++ DI? !D G
e++++ h* r--- y- 
------END GEEK CODE BLOCK------

indeed - the document did not fully survive some of the cleanups.

Because this is all part of the module init code and our purpose here is not locking out other users of the card (which is already accomplished via the fact that card->initializing != 0 at the moment so any other attempts to access the card, which must first go through the open routine to get a file handle to the device, are all blocking in i810_open() or i810_mixer_open() waiting on card->initializing to become 0) but instead it is merely intended to stop all interrupts that might skew our timing via udelay() on the local CPU (it's actually pretty important that we keep our variance from a real 50ms delay as small as possible, since the more variance we allow in this loop the more likely it will be that our sound card will play sounds either a bit too fast or too slow).

  Doug Ledford <[email protected]>     919-754-3700 x44233
         Red Hat, Inc. 
         1801 Varsity Dr.
         Raleigh, NC 27606

how about a disable_irq_all() and enable_irq_all() call, which would disable every single interrupt source in the system? Sure it's a bit heavyweight (it disables the timer interrupt too), but if some driver **really** needs complete silence in the IRQ system then it might be useful. It would roughly be equivalent to cli() and sti(), from the hardirq disabling point of view. [it would not disable bottom halves.]

Ingo

For the precision needed I think a local irq disable and the lock the driver needs itself are sufficient, and the lock _irqsave will handle the IRQ bits

Well, to my belief the irq_all() stuff is overkill as Alan points out. However, Alan also implies that during the init stage we should be holding the card lock and using that to disable interrupts. I disagree with that since we may already have other entry points looking at our card and we don't want the card lock held until after it has been initted and is ready for real use. So, I would leave it just like the patch to fix up the sti usage left it.

  Doug Ledford <[email protected]>     919-754-3700 x44233
         Red Hat, Inc. 
         1801 Varsity Dr.
         Raleigh, NC 27606

The attached patch does the following:

1. Remove blkdev_release_request(Request); it was an unnecessary wrapper around blk_put_request(Request). Likely some leftover from pre-BIO time...

2. Abstract out the fine __scsi_insert_special() function out from the SCSI code.

Now that I have finally managed to kill all those IDE 'specific' REQ_BLAH request types, we can do this final step, and it will be used soon at least by ATA code as well. The goal is that scsi_request_fn and do_ide_request should start to look similar like silblings.

Its called blk_insert_request() now and even documented in code.

3. Change some stuff over from extern inline to static inline in blkdev.h. (trivia...)

This patch doesn't change *any* functionality, so its not exposing SCSI to any danger :-).

Please apply.

Thanks.

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h linux-2.5.28/drivers/block/DAC960.c linux/drivers/block/DAC960.c
@@ -2884,7 +2884,7 @@ static boolean DAC960_ProcessRequest(DAC
   Command->BufferHeader = Request->bio;
   Command->RequestBuffer = Request->buffer;
   blkdev_dequeue_request(Request);
-  blkdev_release_request(Request);
+  blk_put_request(Request);
   DAC960_QueueReadWriteCommand(Command);
   return true;
 }

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h linux-2.5.28/include/linux/blkdev.h linux/include/linux/blkdev.h
@@ -281,12 +281,13 @@ extern int wipe_partitions(kdev_t dev);
 extern void register_disk(struct gendisk *dev, kdev_t first, unsigned minors, struct block_device_operations *ops, long size);
 extern void generic_make_request(struct bio *bio);
 extern inline request_queue_t *bdev_get_queue(struct block_device *bdev);
-extern void blkdev_release_request(struct request *);
+extern void blk_put_request(struct request *);
 extern void blk_attempt_remerge(request_queue_t *, struct request *);
 extern void __blk_attempt_remerge(request_queue_t *, struct request *);
 extern struct request *blk_get_request(request_queue_t *, int, int);
 extern struct request *__blk_get_request(request_queue_t *, int);
 extern void blk_put_request(struct request *);
+extern void blk_insert_request(request_queue_t *, struct request *, int, void *);
 extern void blk_plug_device(request_queue_t *);
 extern int blk_remove_plug(request_queue_t *);
 extern void blk_recount_segments(request_queue_t *, struct bio *);

@@ -309,20 +310,21 @@ extern int blk_init_queue(request_queue_
 extern void blk_cleanup_queue(request_queue_t *);
 extern void blk_queue_make_request(request_queue_t *, make_request_fn *);
 extern void blk_queue_bounce_limit(request_queue_t *, u64);
-extern void blk_queue_max_sectors(request_queue_t *q, unsigned short);
-extern void blk_queue_max_phys_segments(request_queue_t *q, unsigned short);
-extern void blk_queue_max_hw_segments(request_queue_t *q, unsigned short);
-extern void blk_queue_max_segment_size(request_queue_t *q, unsigned int);
-extern void blk_queue_hardsect_size(request_queue_t *q, unsigned short);
-extern void blk_queue_segment_boundary(request_queue_t *q, unsigned long);
-extern void blk_queue_assign_lock(request_queue_t *q, spinlock_t *);
-extern void blk_queue_prep_rq(request_queue_t *q, prep_rq_fn *pfn);
+extern void blk_queue_max_sectors(request_queue_t *, unsigned short);
+extern void blk_queue_max_phys_segments(request_queue_t *, unsigned short);
+extern void blk_queue_max_hw_segments(request_queue_t *, unsigned short);
+extern void blk_queue_max_segment_size(request_queue_t *, unsigned int);
+extern void blk_queue_hardsect_size(request_queue_t *, unsigned short);
+extern void blk_queue_segment_boundary(request_queue_t *, unsigned long);
+extern void blk_queue_assign_lock(request_queue_t *, spinlock_t *);
+extern void blk_queue_prep_rq(request_queue_t *, prep_rq_fn *pfn);
 extern struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev);
 
 extern int blk_rq_map_sg(request_queue_t *, struct request *, struct scatterlist *);
 extern void blk_dump_rq_flags(struct request *, char *);
 extern void generic_unplug_device(void *);
 
+
 /*
  * tag stuff
  */

@@ -348,15 +350,12 @@ extern int * blk_size[MAX_BLKDEV];	/* in
 
 extern void drive_stat_acct(struct request *, int, int);
 
-extern inline void blk_clear(int major)
+static inline void blk_clear(int major)
 {
 	blk_size[major] = NULL;
-#if 0
-	blk_size_in_bytes[major] = NULL;
-#endif
 }
 
-extern inline int queue_hardsect_size(request_queue_t *q)
+static inline int queue_hardsect_size(request_queue_t *q)
 {
 	int retval = 512;
 

@@ -366,7 +365,7 @@ extern inline int queue_hardsect_size(re
 	return retval;
 }
 
-extern inline int bdev_hardsect_size(struct block_device *bdev)
+static inline int bdev_hardsect_size(struct block_device *bdev)
 {
 	return queue_hardsect_size(bdev_get_queue(bdev));
 }

@@ -375,7 +374,7 @@ extern inline int bdev_hardsect_size(str
 #define blk_started_io(nsects)	do { } while (0)
 
 /* assumes size > 256 */
-extern inline unsigned int blksize_bits(unsigned int size)
+static inline unsigned int blksize_bits(unsigned int size)
 {
 	unsigned int bits = 8;
 	do {

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h linux-2.5.28/include/linux/nbd.h linux/include/linux/nbd.h
@@ -61,7 +61,7 @@ nbd_end_request(struct request *req)
 		bio->bi_next = NULL;
 		bio_endio(bio, uptodate);
 	}
-	blkdev_release_request(req);
+	blk_put_request(req);
 	spin_unlock_irqrestore(q->queue_lock, flags);
 }
 

Looks fine to me. One thing sticks out though:

	if (blk_rq_tagged(rq))
		blk_queue_end_tag(q, rq);

	blk_insert_request(q, rq, bla bla);

Jens Axboe

Hey it was *literal* cut and paste from SCSI code after all ;-)

Yes.

Cut and paste damage from SCSI code.... no argument here.

Ah, yes I'm pretty sure now. I looked up how blk_queue_end_tag() works and it's indeed the case -> setting the flag
and undoing it immediately doesn't make sense anyway. (Even the collateral damage to tag allocation aside...) This was perhaps "defensive coding" by the SCSI people?

You are right the

rq->flags &= REQ_QUEUED;

and the

if (blk_rq_tagged(rq))
blk_queue_end_tag(q, rq);

should be just removed and things are fine.
They only survive becouse they don't provide a tag for the request in first place.

Thanks for pointing it out.

But the crap still got merged, sigh... Yet again an excellent point of why stuff like this should go through the maintainer. Apparently Linus blindly applies this stuff.

Jens Axboe

Ehh, since there is no proactive maintainer for SCSI, I don't have much choice, do I?

SCSI has been maintainerless for the last few years. Right now three people work on it to some degree (Doug Ledford, James Bottomley and you), but I don't get timely patches, and neither does apparently anybody else.

Case in point: I was debugging some USB storage issues with Matthew Dharm yesterday, and he sent me patches to the SCSI subsystem that he claims were supposedly considered valid on the scsi mailing list back in May.

Guess what? I've not seen the patches from any of the three people I consider closest to being maintainers.

So your "should go through the maintainer" complaint is obviously a bunch of bull. Feel free to step up to the plate, but before you do, don't throw rocks in glass houses.

Linus

SCSI is always the first to get neglected it seems, and yes I'm guilty of that as well. Maybe that can change in the future.

I was referring to the block layer, not the SCSI layer. The broken changes were applied to the block layer after all, I had not even noticed that the SCSI one was broken.

Jens Axboe

Heh.

Anyway, I don't think the situation is "wrong" per se. Martin took code that was generic from SCSI, and moved it to the common place. In the process, you noticed that the original code was broken. Downsides? I guess it gets fixed now. Sounds reasonable to me, and we should just be happy that these things get noticed eventually, even if the reason for noticing it is the "wrong" one.

Linus

Jens. Please note that this doesn't make *anything* worser then before, since I don't use this function right now.

SCSI does, though :-)

It's ok now, the issue is resolved as far as I'm concerned.

Jens Axboe

Fine. So I will start to use it soon in IDE too...

Just getting trivia out of the way, so the interresting parts don't get burried by them:

- Sanitize the menu configuration system.

- Allow to compile atapi.c as a "foundation module" for the consuming device type drivers.

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/alpha/config.in linux/arch/alpha/config.in
@@ -283,9 +283,9 @@ if [ "$CONFIG_NET" = "y" ]; then
 fi
 
 mainmenu_option next_comment
-comment 'ATA/IDE/MFM/RLL support'
+comment 'ATA/ATAPI/MFM/RLL support'
 
-tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+tristate 'ATA/ATAPI/MFM/RLL support' CONFIG_IDE
 
 if [ "$CONFIG_IDE" != "n" ]; then
   source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/arm/config.in linux/arch/arm/config.in
@@ -529,9 +529,9 @@ if [ "$CONFIG_NET" = "y" ]; then
 fi
 
 mainmenu_option next_comment
-comment 'ATA/IDE/MFM/RLL support'
+comment 'ATA/ATAPI/MFM/RLL support'
 
-tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+tristate 'ATA/ATAPI/MFM/RLL support' CONFIG_IDE
 
 if [ "$CONFIG_IDE" != "n" ]; then
   source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/ia64/config.in linux/arch/ia64/config.in
@@ -133,9 +133,9 @@ source drivers/md/Config.in
 source drivers/message/fusion/Config.in
 
 mainmenu_option next_comment
-comment 'ATA/IDE/MFM/RLL support'
+comment 'ATA/ATAPI/MFM/RLL support'
 
-tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+tristate 'ATA/ATAPI/MFM/RLL support' CONFIG_IDE
 
 if [ "$CONFIG_IDE" != "n" ]; then
   source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/m68k/config.in linux/arch/m68k/config.in
@@ -160,9 +160,9 @@ if [ "$CONFIG_MAC" = "y" ]; then
 fi
 
 mainmenu_option next_comment
-comment 'ATA/IDE/MFM/RLL support'
+comment 'ATA/ATAPI/MFM/RLL device support'
 
-tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+tristate 'ATA/ATAPI/MFM/RLL device support' CONFIG_IDE
 
 if [ "$CONFIG_IDE" != "n" ]; then
   source drivers/ide/Config.in

@@ -172,9 +172,9 @@ fi
 endmenu
 
 mainmenu_option next_comment
-comment 'SCSI support'
+comment 'SCSI device support'
 
-tristate 'SCSI support' CONFIG_SCSI
+tristate 'SCSI device support' CONFIG_SCSI
 
 if [ "$CONFIG_SCSI" != "n" ]; then
 

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/mips/config.in linux/arch/mips/config.in
@@ -343,9 +343,9 @@ if [ "$CONFIG_SGI_IP22" != "y" -a \
      "$CONFIG_DECSTATION" != "y" ]; then
 
    mainmenu_option next_comment
-   comment 'ATA/IDE/MFM/RLL support'
+   comment 'ATA/ATAPI/MFM/RLL support'
     
-   tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+   tristate 'ATA/ATAPI/MFM/RLL support' CONFIG_IDE
     
    if [ "$CONFIG_IDE" != "n" ]; then
       source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/mips64/config.in linux/arch/mips64/config.in
@@ -138,9 +138,9 @@ fi
 source drivers/telephony/Config.in
 
 mainmenu_option next_comment
-comment 'ATA/IDE/MFM/RLL support'
+comment 'ATA/ATAPI/MFM/RLL support'
 
-tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+tristate 'ATA/ATAPI/MFM/RLL support' CONFIG_IDE
 
 if [ "$CONFIG_IDE" != "n" ]; then
   source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/ppc64/config.in linux/arch/ppc64/config.in
@@ -92,9 +92,9 @@ if [ "$CONFIG_NET" = "y" ]; then
 fi
 
 mainmenu_option next_comment
-comment 'ATA/IDE/MFM/RLL support'
+comment 'ATA/ATAPI/MFM/RLL support'
 
-tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+tristate 'ATA/ATAPI/MFM/RLL support' CONFIG_IDE
 
 if [ "$CONFIG_IDE" != "n" ]; then
   source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/sh/config.in linux/arch/sh/config.in
@@ -211,9 +211,9 @@ if [ "$CONFIG_NET" = "y" ]; then
 fi
 
 mainmenu_option next_comment
-comment 'ATA/IDE/MFM/RLL support'
+comment 'ATA/ATAPI/MFM/RLL support'
 
-tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+tristate 'ATA/ATAPI/MFM/RLL support' CONFIG_IDE
 
 if [ "$CONFIG_IDE" != "n" ]; then
   source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/sparc/config.in linux/arch/sparc/config.in
@@ -104,9 +104,9 @@ fi
 if [ "$CONFIG_PCI" = "y" ]; then
 
   mainmenu_option next_comment
-  comment 'ATA/IDE/MFM/RLL support'
+  comment 'ATA/ATAPI/MFM/RLL support'
 
-  tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+  tristate 'ATA/ATAPI/MFM/RLL support' CONFIG_IDE
 
   if [ "$CONFIG_IDE" != "n" ]; then
     source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/sparc64/config.in linux/arch/sparc64/config.in
@@ -105,9 +105,9 @@ if [ "$CONFIG_NET" = "y" ]; then
 fi
 
 mainmenu_option next_comment
-comment 'ATA/IDE/MFM/RLL support'
+comment 'ATA/ATAPI/MFM/RLL device support'
 
-tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+tristate 'ATA/ATAPI/MFM/RLL device support' CONFIG_IDE
 
 if [ "$CONFIG_IDE" != "n" ]; then
   source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/arch/x86_64/config.in linux/arch/x86_64/config.in
@@ -126,9 +126,9 @@ fi
 source drivers/telephony/Config.in
 
 mainmenu_option next_comment
-comment 'ATA/IDE/MFM/RLL support'
+comment 'ATA/ATAPI/MFM/RLL support'
 
-tristate 'ATA/IDE/MFM/RLL support' CONFIG_IDE
+tristate 'ATA/ATAPI/MFM/RLL support' CONFIG_IDE
 
 if [ "$CONFIG_IDE" != "n" ]; then
   source drivers/ide/Config.in

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/drivers/ide/Config.help linux/drivers/ide/Config.help
@@ -84,6 +84,16 @@ CONFIG_BLK_DEV_IDECS
   Support for outboard IDE disks, tape drives, and CD-ROM drives
   connected through a  PCMCIA card.
 
+CONFIG_ATAPI
+  If you wish to enable basic support for devices attached to the system 
+  through the ATA interface, and which are using using the ATAPI protocol
+  (CD-ROM, CD-RW, DVD, DVD-RW, LS120, ZIP, ...), say Y.
+  
+  If you want to compile the driver as a module ( = code which can be
+  inserted in and removed from the running kernel whenever you want),
+  say M here and read <file:Documentation/modules.txt>.  The module
+  will be called atapi.o.
+  
 CONFIG_BLK_DEV_IDECD
   If you have a CD-ROM drive using the ATAPI protocol, say Y. ATAPI is
   a newer protocol used by IDE CD-ROM and TAPE drives, similar to the

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/drivers/ide/Makefile linux/drivers/ide/Makefile
@@ -15,7 +15,7 @@ obj-$(CONFIG_BLK_DEV_HD)	+= hd.o
 obj-$(CONFIG_BLK_DEV_IDE)       += ide-mod.o
 obj-$(CONFIG_BLK_DEV_IDECS)     += ide-cs.o
 obj-$(CONFIG_BLK_DEV_IDEDISK)   += ide-disk.o
-# obj-$(CONFIG_ATAPI)		+= atapi.o
+obj-$(CONFIG_ATAPI)		+= atapi.o
 obj-$(CONFIG_BLK_DEV_IDECD)     += ide-cd.o
 obj-$(CONFIG_BLK_DEV_IDETAPE)   += ide-tape.o
 obj-$(CONFIG_BLK_DEV_IDEFLOPPY) += ide-floppy.o

@@ -69,6 +69,6 @@ obj-$(CONFIG_BLK_DEV_ATARAID_PDC)	+= pdc
 obj-$(CONFIG_BLK_DEV_ATARAID_HPT)	+= hptraid.o
 
 ide-mod-objs	:= device.o ide-taskfile.o main.o ide.o probe.o \
-		   ioctl.o atapi.o ata-timing.o $(ide-obj-y)
+		   ioctl.o ata-timing.o $(ide-obj-y)
 
 include $(TOPDIR)/Rules.make

- Remove pseudo headers for nonexisting support of not existing hardware from Big Black Boxen code.

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/include/asm-s390/hdreg.h linux/include/asm-s390/hdreg.h
@@ -1,13 +0,0 @@
-/*
- *  linux/include/asm-arm/hdreg.h
- *
- *  Copyright (C) 1994-1996  Linus Torvalds & authors
- */
-
-#ifndef __ASMS390_HDREG_H
-#define __ASMS390_HDREG_H
-
-typedef unsigned long ide_ioreg_t;
-
-#endif /* __ASMS390_HDREG_H */
-

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/include/asm-s390/ide.h linux/include/asm-s390/ide.h
@@ -1,27 +0,0 @@
-/*
- *  linux/include/asm-s390/ide.h
- *
- *  Copyright (C) 1994-1996  Linus Torvalds & authors
- */
-
-/* s390 does not have IDE */
-
-#ifndef __ASMS390_IDE_H
-#define __ASMS390_IDE_H
-
-#ifdef __KERNEL__
-
-#ifndef MAX_HWIFS
-#define MAX_HWIFS	0
-#endif
-
-/*
- * We always use the new IDE port registering,
- * so these are fixed here.
- */
-#define ide_default_io_base(i)		((ide_ioreg_t)0)
-#define ide_default_irq(b)		(0)
-
-#endif /* __KERNEL__ */
-
-#endif /* __ASMS390_IDE_H */

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/include/asm-s390x/hdreg.h linux/include/asm-s390x/hdreg.h
@@ -1,13 +0,0 @@
-/*
- *  linux/include/asm-arm/hdreg.h
- *
- *  Copyright (C) 1994-1996  Linus Torvalds & authors
- */
-
-#ifndef __ASMS390_HDREG_H
-#define __ASMS390_HDREG_H
-
-typedef unsigned long ide_ioreg_t;
-
-#endif /* __ASMS390_HDREG_H */
-

- Make the bit-sliced data types in hdreg.h use the bit-slice data types instead of the generic ones. This makes clear that those are supposed to be register masks.

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/include/linux/hdreg.h linux/include/linux/hdreg.h
@@ -261,17 +261,17 @@ struct hd_drive_task_hdr {
 #define SECURITY_DISABLE_PASSWORD	0xBF
 
 struct hd_geometry {
-      unsigned char heads;
-      unsigned char sectors;
-      unsigned short cylinders;
+      u8 heads;
+      u8 sectors;
+      u16 cylinders;
       unsigned long start;
 };
 
 /* BIG GEOMETRY - dying, used only by HDIO_GETGEO_BIG_RAW */
 struct hd_big_geometry {
-	unsigned char heads;
-	unsigned char sectors;
-	unsigned int cylinders;
+	u8 heads;
+	u8 sectors;
+	u32 cylinders;
 	unsigned long start;
 };
 

@@ -326,249 +326,243 @@ enum {
  * ide/probe.c.
  */
 struct hd_driveid {
-	unsigned short	config;		/* lots of obsolete bit flags */
-	unsigned short	cyls;		/* Obsolete, "physical" cyls */
-	unsigned short	reserved2;	/* reserved (word 2) */
-	unsigned short	heads;		/* Obsolete, "physical" heads */
-	unsigned short	track_bytes;	/* unformatted bytes per track */
-	unsigned short	sector_bytes;	/* unformatted bytes per sector */
-	unsigned short	sectors;	/* Obsolete, "physical" sectors per track */
-	unsigned short	vendor0;	/* vendor unique */
-	unsigned short	vendor1;	/* vendor unique */
-	unsigned short	vendor2;	/* Retired vendor unique */
-	unsigned char	serial_no[20];	/* 0 = not_specified */
-	unsigned short	buf_type;	/* Retired */
-	unsigned short	buf_size;	/* Retired, 512 byte increments
-					 * 0 = not_specified
-					 */
-	unsigned short	ecc_bytes;	/* for r/w long cmds; 0 = not_specified */
-	unsigned char	fw_rev[8];	/* 0 = not_specified */
-	unsigned char	model[40];	/* 0 = not_specified */
-	unsigned char	max_multsect;	/* 0=not_implemented */
-	unsigned char	vendor3;	/* vendor unique */
-	unsigned short	dword_io;	/* 0=not_implemented; 1=implemented */
-	unsigned char	vendor4;	/* vendor unique */
-	unsigned char	capability;	/* (upper byte of word 49)
-					 *  3:	IORDYsup
-					 *  2:	IORDYsw
-					 *  1:	LBA
-					 *  0:	DMA
-					 */
-	unsigned short	reserved50;	/* reserved (word 50) */
-	unsigned char	vendor5;	/* Obsolete, vendor unique */
-	unsigned char	tPIO;		/* Obsolete, 0=slow, 1=medium, 2=fast */
-	unsigned char	vendor6;	/* Obsolete, vendor unique */
-	unsigned char	tDMA;		/* Obsolete, 0=slow, 1=medium, 2=fast */
-	unsigned short	field_valid;	/* (word 53)
-					 *  2:	ultra_ok	word  88
-					 *  1:	eide_ok		words 64-70
-					 *  0:	cur_ok		words 54-58
-					 */
-	unsigned short	cur_cyls;	/* Obsolete, logical cylinders */
-	unsigned short	cur_heads;	/* Obsolete, l heads */
-	unsigned short	cur_sectors;	/* Obsolete, l sectors per track */
-	unsigned short	cur_capacity0;	/* Obsolete, l total sectors on drive */
-	unsigned short	cur_capacity1;	/* Obsolete, (2 words, misaligned int)     */
-	unsigned char	multsect;	/* current multiple sector count */
-	unsigned char	multsect_valid;	/* when (bit0==1) multsect is ok */
-	unsigned int	lba_capacity;	/* Obsolete, total number of sectors */
-	unsigned short	dma_1word;	/* Obsolete, single-word dma info */
-	unsigned short	dma_mword;	/* multiple-word dma info */
-	unsigned short  eide_pio_modes; /* bits 0:mode3 1:mode4 */
-	unsigned short  eide_dma_min;	/* min mword dma cycle time (ns) */
-	unsigned short  eide_dma_time;	/* recommended mword dma cycle time (ns) */
-	unsigned short  eide_pio;       /* min cycle time (ns), no IORDY  */
-	unsigned short  eide_pio_iordy; /* min cycle time (ns), with IORDY */
-	unsigned short	words69_70[2];	/* reserved words 69-70
-					 * future command overlap and queuing
-					 */
+	u16	config;		/* lots of obsolete bit flags */
+	u16	cyls;		/* Obsolete, "physical" cyls */
+	u16	reserved2;	/* reserved (word 2) */
+	u16	heads;		/* Obsolete, "physical" heads */
+	u16	track_bytes;	/* unformatted bytes per track */
+	u16	sector_bytes;	/* unformatted bytes per sector */
+	u16	sectors;	/* Obsolete, "physical" sectors per track */
+	u16	vendor0;	/* vendor unique */
+	u16	vendor1;	/* vendor unique */
+	u16	vendor2;	/* Retired vendor unique */
+	u8	serial_no[20];	/* 0 = not_specified */
+	u16	buf_type;	/* Retired */
+	u16	buf_size;	/* Retired, 512 byte increments
+				 * 0 = not_specified
+				 */
+	u16	ecc_bytes;	/* for r/w long cmds; 0 = not_specified */
+	u8	fw_rev[8];	/* 0 = not_specified */
+	char	model[40];	/* 0 = not_specified */
+	u8	max_multsect;	/* 0=not_implemented */
+	u8	vendor3;	/* vendor unique */
+	u16	dword_io;	/* 0=not_implemented; 1=implemented */
+	u8	vendor4;	/* vendor unique */
+	u8	capability;	/* (upper byte of word 49)
+				 *  3:	IORDYsup
+				 *  2:	IORDYsw
+				 *  1:	LBA
+				 *  0:	DMA
+				 */
+	u16	reserved50;	/* reserved (word 50) */
+	u8	vendor5;	/* Obsolete, vendor unique */
+	u8	tPIO;		/* Obsolete, 0=slow, 1=medium, 2=fast */
+	u8	vendor6;	/* Obsolete, vendor unique */
+	u8	tDMA;		/* Obsolete, 0=slow, 1=medium, 2=fast */
+	u16	field_valid;	/* (word 53)
+				 *  2:	ultra_ok	word  88
+				 *  1:	eide_ok		words 64-70
+				 *  0:	cur_ok		words 54-58
+				 */
+	u16	cur_cyls;	/* Obsolete, logical cylinders */
+	u16	cur_heads;	/* Obsolete, l heads */
+	u16	cur_sectors;	/* Obsolete, l sectors per track */
+	u16	cur_capacity0;	/* Obsolete, l total sectors on drive */
+	u16	cur_capacity1;	/* Obsolete, (2 words, misaligned int)     */
+	u8	multsect;	/* current multiple sector count */
+	u8	multsect_valid;	/* when (bit0==1) multsect is ok */
+	u32	lba_capacity;	/* Obsolete, total number of sectors */
+	u16	dma_1word;	/* Obsolete, single-word dma info */
+	u16	dma_mword;	/* multiple-word dma info */
+	u16	eide_pio_modes; /* bits 0:mode3 1:mode4 */
+	u16	eide_dma_min;	/* min mword dma cycle time (ns) */
+	u16	eide_dma_time;	/* recommended mword dma cycle time (ns) */
+	u16	eide_pio;       /* min cycle time (ns), no IORDY  */
+	u16	eide_pio_iordy; /* min cycle time (ns), with IORDY */
+	u16	words69_70[2];	/* reserved words 69-70
+				 * future command overlap and queuing
+				 */
 	/* HDIO_GET_IDENTITY currently returns only words 0 through 70 */
-	unsigned short	words71_74[4];	/* reserved words 71-74
-					 * for IDENTIFY PACKET DEVICE command
-					 */
-	unsigned short  queue_depth;	/* (word 75)
-					 * 15:5	reserved
-					 *  4:0	Maximum queue depth -1
-					 */
-	unsigned short  words76_79[4];	/* reserved words 76-79 */
-	unsigned short  major_rev_num;	/* (word 80) */
-	unsigned short  minor_rev_num;	/* (word 81) */
-	unsigned short  command_set_1;	/* (word 82) supported
-					 * 15:	Obsolete
-					 * 14:	NOP command
-					 * 13:	READ_BUFFER
-					 * 12:	WRITE_BUFFER
-					 * 11:	Obsolete
-					 * 10:	Host Protected Area
-					 *  9:	DEVICE Reset
-					 *  8:	SERVICE Interrupt
-					 *  7:	Release Interrupt
-					 *  6:	look-ahead
-					 *  5:	write cache
-					 *  4:	PACKET Command
-					 *  3:	Power Management Feature Set
-					 *  2:	Removable Feature Set
-					 *  1:	Security Feature Set
-					 *  0:	SMART Feature Set
-					 */
-	unsigned short  command_set_2;	/* (word 83)
-					 * 15:	Shall be ZERO
-					 * 14:	Shall be ONE
-					 * 13:	FLUSH CACHE EXT
-					 * 12:	FLUSH CACHE
-					 * 11:	Device Configuration Overlay
-					 * 10:	48-bit Address Feature Set
-					 *  9:	Automatic Acoustic Management
-					 *  8:	SET MAX security
-					 *  7:	reserved 1407DT PARTIES
-					 *  6:	SetF sub-command Power-Up
-					 *  5:	Power-Up in Standby Feature Set
-					 *  4:	Removable Media Notification
-					 *  3:	APM Feature Set
-					 *  2:	CFA Feature Set
-					 *  1:	READ/WRITE DMA QUEUED
-					 *  0:	Download MicroCode
-					 */
-	unsigned short  cfsse;		/* (word 84)
-					 * cmd set-feature supported extensions
-					 * 15:	Shall be ZERO
-					 * 14:	Shall be ONE
-					 * 13:3	reserved
-					 *  2:	Media Serial Number Valid
-					 *  1:	SMART selt-test supported
-					 *  0:	SMART error logging
-					 */
-	unsigned short  cfs_enable_1;	/* (word 85)
-					 * command set-feature enabled
-					 * 15:	Obsolete
-					 * 14:	NOP command
-					 * 13:	READ_BUFFER
-					 * 12:	WRITE_BUFFER
-					 * 11:	Obsolete
-					 * 10:	Host Protected Area
-					 *  9:	DEVICE Reset
-					 *  8:	SERVICE Interrupt
-					 *  7:	Release Interrupt
-					 *  6:	look-ahead
-					 *  5:	write cache
-					 *  4:	PACKET Command
-					 *  3:	Power Management Feature Set
-					 *  2:	Removable Feature Set
-					 *  1:	Security Feature Set
-					 *  0:	SMART Feature Set
-					 */
-	unsigned short  cfs_enable_2;	/* (word 86)
-					 * command set-feature enabled
-					 * 15:	Shall be ZERO
-					 * 14:	Shall be ONE
-					 * 13:	FLUSH CACHE EXT
-					 * 12:	FLUSH CACHE
-					 * 11:	Device Configuration Overlay
-					 * 10:	48-bit Address Feature Set
-					 *  9:	Automatic Acoustic Management
-					 *  8:	SET MAX security
-					 *  7:	reserved 1407DT PARTIES
-					 *  6:	SetF sub-command Power-Up
-					 *  5:	Power-Up in Standby Feature Set
-					 *  4:	Removable Media Notification
-					 *  3:	APM Feature Set
-					 *  2:	CFA Feature Set
-					 *  1:	READ/WRITE DMA QUEUED
-					 *  0:	Download MicroCode
-					 */
-	unsigned short  csf_default;	/* (word 87)
-					 * command set-feature default
-					 * 15:	Shall be ZERO
-					 * 14:	Shall be ONE
-					 * 13:3	reserved
-					 *  2:	Media Serial Number Valid
-					 *  1:	SMART selt-test supported
-					 *  0:	SMART error logging
-					 */
-	unsigned short  dma_ultra;	/* (word 88) */
-	unsigned short	word89;		/* reserved (word 89) */
-	unsigned short	word90;		/* reserved (word 90) */
-	unsigned short	CurAPMvalues;	/* current APM values */
-	unsigned short	word92;		/* reserved (word 92) */
-	unsigned short	hw_config;	/* hardware config (word 93)
-					 * 15:
-					 * 14:
-					 * 13:
-					 * 12:
-					 * 11:
-					 * 10:
-					 *  9:
-					 *  8:
-					 *  7:
-					 *  6:
-					 *  5:
-					 *  4:
-					 *  3:
-					 *  2:
-					 *  1:
-					 *  0:
-					 */
-	unsigned short	acoustic;	/* (word 94)
-					 * 15:8	Vendor's recommended value
-					 *  7:0	current value
-					 */
-	unsigned short	words95_99[5];	/* reserved words 95-99 */
-	unsigned long long lba_capacity_2;/* 48-bit total number of sectors */
-	unsigned short	words104_125[22];/* reserved words 104-125 */
-	unsigned short	last_lun;	/* (word 126) */
-	unsigned short	word127;	/* (word 127) Feature Set
-					 * Removable Media Notification
-					 * 15:2	reserved
-					 *  1:0	00 = not supported
-					 *	01 = supported
-					 *	10 = reserved
-					 *	11 = reserved
-					 */
-	unsigned short	dlf;		/* (word 128)
-					 * device lock function
-					 * 15:9	reserved
-					 *  8	security level 1:max 0:high
-					 *  7:6	reserved
-					 *  5	enhanced erase
-					 *  4	expire
-					 *  3	frozen
-					 *  2	locked
-					 *  1	en/disabled
-					 *  0	capability
-					 */
-	unsigned short  csfo;		/*  (word 129)
-					 * current set features options
-					 * 15:4	reserved
-					 *  3:	auto reassign
-					 *  2:	reverting
-					 *  1:	read-look-ahead
-					 *  0:	write cache
-					 */
-	unsigned short	words130_155[26];/* reserved vendor words 130-155 */
-	unsigned short	word156;	/* reserved vendor word 156 */
-	unsigned short	words157_159[3];/* reserved vendor words 157-159 */
-	unsigned short	cfa_power;	/* (word 160) CFA Power Mode
-					 * 15 word 160 supported
-					 * 14 reserved
-					 * 13
-					 * 12
-					 * 11:0
-					 */
-	unsigned short	words161_175[14];/* Reserved for CFA */
-	unsigned short	words176_205[31];/* Current Media Serial Number */
-	unsigned short	words206_254[48];/* reserved words 206-254 */
-	unsigned short	integrity_word;	/* (word 255)
-					 * 15:8 Checksum
-					 *  7:0 Signature
-					 */
+	u16	words71_74[4];	/* reserved words 71-74
+				 * for IDENTIFY PACKET DEVICE command
+				 */
+	u16	queue_depth;	/* (word 75)
+				 * 15:5	reserved
+				 *  4:0	Maximum queue depth -1
+				 */
+	u16	words76_79[4];	/* reserved words 76-79 */
+	u16	major_rev_num;	/* (word 80) */
+	u16	minor_rev_num;	/* (word 81) */
+	u16	command_set_1;	/* (word 82) supported
+				 * 15:	Obsolete
+				 * 14:	NOP command
+				 * 13:	READ_BUFFER
+				 * 12:	WRITE_BUFFER
+				 * 11:	Obsolete
+				 * 10:	Host Protected Area
+				 *  9:	DEVICE Reset
+				 *  8:	SERVICE Interrupt
+				 *  7:	Release Interrupt
+				 *  6:	look-ahead
+				 *  5:	write cache
+				 *  4:	PACKET Command
+				 *  3:	Power Management Feature Set
+				 *  2:	Removable Feature Set
+				 *  1:	Security Feature Set
+				 *  0:	SMART Feature Set
+				 */
+	u16	command_set_2;	/* (word 83)
+				 * 15:	Shall be ZERO
+				 * 14:	Shall be ONE
+				 * 13:	FLUSH CACHE EXT
+				 * 12:	FLUSH CACHE
+				 * 11:	Device Configuration Overlay
+				 * 10:	48-bit Address Feature Set
+				 *  9:	Automatic Acoustic Management
+				 *  8:	SET MAX security
+				 *  7:	reserved 1407DT PARTIES
+				 *  6:	SetF sub-command Power-Up
+				 *  5:	Power-Up in Standby Feature Set
+				 *  4:	Removable Media Notification
+				 *  3:	APM Feature Set
+				 *  2:	CFA Feature Set
+				 *  1:	READ/WRITE DMA QUEUED
+				 *  0:	Download MicroCode
+				 */
+	u16	cfsse;		/* (word 84)
+				 * cmd set-feature supported extensions
+				 * 15:	Shall be ZERO
+				 * 14:	Shall be ONE
+				 * 13:3	reserved
+				 *  2:	Media Serial Number Valid
+				 *  1:	SMART selt-test supported
+				 *  0:	SMART error logging
+				 */
+	u16	cfs_enable_1;	/* (word 85)
+				 * command set-feature enabled
+				 * 15:	Obsolete
+				 * 14:	NOP command
+				 * 13:	READ_BUFFER
+				 * 12:	WRITE_BUFFER
+				 * 11:	Obsolete
+				 * 10:	Host Protected Area
+				 *  9:	DEVICE Reset
+				 *  8:	SERVICE Interrupt
+				 *  7:	Release Interrupt
+				 *  6:	look-ahead
+				 *  5:	write cache
+				 *  4:	PACKET Command
+				 *  3:	Power Management Feature Set
+				 *  2:	Removable Feature Set
+				 *  1:	Security Feature Set
+				 *  0:	SMART Feature Set
+				 */
+	u16	cfs_enable_2;	/* (word 86)
+				 * command set-feature enabled
+				 * 15:	Shall be ZERO
+				 * 14:	Shall be ONE
+				 * 13:	FLUSH CACHE EXT
+				 * 12:	FLUSH CACHE
+				 * 11:	Device Configuration Overlay
+				 * 10:	48-bit Address Feature Set
+				 *  9:	Automatic Acoustic Management
+				 *  8:	SET MAX security
+				 *  7:	reserved 1407DT PARTIES
+				 *  6:	SetF sub-command Power-Up
+				 *  5:	Power-Up in Standby Feature Set
+				 *  4:	Removable Media Notification
+				 *  3:	APM Feature Set
+				 *  2:	CFA Feature Set
+				 *  1:	READ/WRITE DMA QUEUED
+				 *  0:	Download MicroCode
+				 */
+	u16	csf_default;	/* (word 87)
+				 * command set-feature default
+				 * 15:	Shall be ZERO
+				 * 14:	Shall be ONE
+				 * 13:3	reserved
+				 *  2:	Media Serial Number Valid
+				 *  1:	SMART selt-test supported
+				 *  0:	SMART error logging
+				 */
+	u16	dma_ultra;	/* (word 88) */
+	u16	word89;		/* reserved (word 89) */
+	u16	word90;		/* reserved (word 90) */
+	u16	CurAPMvalues;	/* current APM values */
+	u16	word92;		/* reserved (word 92) */
+	u16	hw_config;	/* hardware config (word 93)
+				 * 15:
+				 * 14:
+				 * 13:
+				 * 12:
+				 * 11:
+				 * 10:
+				 *  9:
+				 *  8:
+				 *  7:
+				 *  6:
+				 *  5:
+				 *  4:
+				 *  3:
+				 *  2:
+				 *  1:
+				 *  0:
+				 */
+	u16	acoustic;	/* (word 94)
+				 * 15:8	Vendor's recommended value
+				 *  7:0	current value
+				 */
+	u16	words95_99[5];	/* reserved words 95-99 */
+	u64	lba_capacity_2;	/* 48-bit total number of sectors */
+	u16	words104_125[22];/* reserved words 104-125 */
+	u16	last_lun;	/* (word 126) */
+	u16	word127;	/* (word 127) Feature Set
+				 * Removable Media Notification
+				 * 15:2	reserved
+				 *  1:0	00 = not supported
+				 *	01 = supported
+				 *	10 = reserved
+				 *	11 = reserved
+				 */
+	u16	dlf;		/* (word 128)
+				 * device lock function
+				 * 15:9	reserved
+				 *  8	security level 1:max 0:high
+				 *  7:6	reserved
+				 *  5	enhanced erase
+				 *  4	expire
+				 *  3	frozen
+				 *  2	locked
+				 *  1	en/disabled
+				 *  0	capability
+				 */
+	u16	csfo;		/* (word 129)
+				 * current set features options
+				 * 15:4	reserved
+				 *  3:	auto reassign
+				 *  2:	reverting
+				 *  1:	read-look-ahead
+				 *  0:	write cache
+				 */
+	u16	words130_155[26];/* reserved vendor words 130-155 */
+	u16	word156;	/* reserved vendor word 156 */
+	u16	words157_159[3];/* reserved vendor words 157-159 */
+	u16	cfa_power;	/* (word 160) CFA Power Mode
+				 * 15 word 160 supported
+				 * 14 reserved
+				 * 13
+				 * 12
+				 * 11:0
+				 */
+	u16	words161_175[14];/* Reserved for CFA */
+	u16	words176_205[31];/* Current Media Serial Number */
+	u16	words206_254[48];/* reserved words 206-254 */
+	u16	integrity_word;	/* (word 255)
+				 * 15:8 Checksum
+				 *  7:0 Signature
+				 */
 } __attribute__((packed));
 
-/*
- * IDE "nice" flags. These are used on a per drive basis to determine
- * when to be nice and give more bandwidth to the other devices which
- * share the same IDE bus.
- */
 #define IDE_NICE_DSC_OVERLAP	(0)	/* per the DSC overlap protocol */
-#define IDE_NICE_ATAPI_OVERLAP	(1)	/* not supported yet */
 
-#endif	/* _LINUX_HDREG_H */
+#endif

Because its an external API it needs to remain constant. Swapping uchar/uint looks fine and the structure is off the drive anyway. However they should be __u16 __u8 __u32 so that C libraries and apps can still use the header (its __u16 because 'u16' cannot be exposed to user space directly or via libc)

Alan

???

The only thing which is really 'external API' there is the declaration of the HDIO_XXX ioctl and among them in reality only HDIO_GETGEO
is really used outside the scope of the dreddy hdparm application.
And
99% of times its usage is bogous anyway. Or do you know any better examples I'm not aware of?

The remainings will be moved away from there soon becouse it doesn't
make any sense to include this at every single place out there where
HDIO_GETGEO is the needed declaration. If some application needs ATA command macros - it should come with they own header declaring them instead of peeing in to the kernel becouse they don't depend on the kernel in question. They are cut in stone at www.t13.org ;-).

I know - see above.

The struct hd_geometry is used by ioctl HDIO_GETGEO. The struct hd_driveid is used by ioctl HDIO_GETIDENTITY

This would be great, right now lots of drivers suck in the file for the GETGEO stuff even though they are nothing to do with st506 or ide.

Alan

:-). That's the intention indeed behind the above game. Please look at IDE patch nr 105. There is a change to the IDE parts to include hdparm.h explicitely. The next change will be to let them include a new header containing the definitions of ATA commands. This time I will remove the ATA stuff from hdparm.h In hdparm only the declarations of the ioctl stuff will with the two structures will remain. At this point I will throw the double underscores in front of the uXX-es to suite you.

OK?

- Rename ata-timings.h to timings.h. Same arguments as for agp.

- Always include hdparm.h just before ide.h. Include them last where used. This is preparing to split out the IDE register declarations out of this file, since many other files in the kernel include it, which don't have anything to do with IDE.

- Don't use the "IDE special" data type "byte". Just use the u8 data type for consistency with the rest of the kernel where applicable.

Small missing notch.

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/drivers/ide/main.c linux/drivers/ide/main.c
@@ -1069,7 +1069,6 @@ int ide_register_subdriver(struct ata_de
 
 	}
 	drive->revalidate = 1;
-	drive->suspend_reset = 0;
 
 	return 0;
 }

- Fix "temporal anomaly" in do_ide_request pointed out by Petr Vandrovec. Thanks Petr!

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/drivers/ide/ide.c linux/drivers/ide/ide.c
@@ -517,258 +517,251 @@ void ide_stall_queue(struct ata_device *
  * Issue a new request.
  * Caller must have already done spin_lock_irqsave(channel->lock, ...)
  */
-static void do_request(struct ata_channel *channel)
+void do_ide_request(request_queue_t *q)
 {
-	struct ata_channel *ch;
-	struct ata_device *drive = NULL;
-	unsigned int unit;
-	ide_startstop_t ret;
+	struct ata_channel *channel = q->queuedata;
 
-	local_irq_disable();	/* necessary paranoia */
+	while (!test_and_set_bit(IDE_BUSY, channel->active)) {
+		struct ata_channel *ch;
+		struct ata_device *drive = NULL;
+		unsigned int unit;
+		ide_startstop_t ret;
 
-	/*
-	 * Select the next device which will be serviced.  This selects
-	 * only between devices on the same channel, since everything
-	 * else will be scheduled on the queue level.
-	 */
+		/*
+		 * Select the next device which will be serviced.  This selects
+		 * only between devices on the same channel, since everything
+		 * else will be scheduled on the queue level.
+		 */
 
-	for (unit = 0; unit < MAX_DRIVES; ++unit) {
-		struct ata_device *tmp = &channel->drives[unit];
+		for (unit = 0; unit < MAX_DRIVES; ++unit) {
+			struct ata_device *tmp = &channel->drives[unit];
 
-		if (!tmp->present)
-			continue;
+			if (!tmp->present)
+				continue;
 
-		/* There are no requests pending for this device.
-		 */
-		if (blk_queue_empty(&tmp->queue))
-			continue;
+			/* There are no requests pending for this device.
+			 */
+			if (blk_queue_empty(&tmp->queue))
+				continue;
 
 
-		/* This device still wants to remain idle.
-		 */
-		if (tmp->sleep && time_after(tmp->sleep, jiffies))
-			continue;
+			/* This device still wants to remain idle.
+			 */
+			if (tmp->sleep && time_after(tmp->sleep, jiffies))
+				continue;
 
-		/* Take this device, if there is no device choosen thus
-		 * far or which is more urgent.
-		 */
-		if (!drive || (tmp->sleep && (!drive->sleep || time_after(drive->sleep, tmp->sleep)))) {
-			if (!blk_queue_plugged(&tmp->queue))
-				drive = tmp;
+			/* Take this device, if there is no device choosen thus
+			 * far or which is more urgent.
+			 */
+			if (!drive || (tmp->sleep && (!drive->sleep || time_after(drive->sleep, tmp->sleep)))) {
+				if (!blk_queue_plugged(&tmp->queue))
+					drive = tmp;
+			}
 		}
-	}
 
-	if (!drive) {
-		unsigned long sleep = 0;
+		if (!drive) {
+			unsigned long sleep = 0;
 
-		for (unit = 0; unit < MAX_DRIVES; ++unit) {
-			struct ata_device *tmp = &channel->drives[unit];
+			for (unit = 0; unit < MAX_DRIVES; ++unit) {
+				struct ata_device *tmp = &channel->drives[unit];
 
-			if (!tmp->present)
-				continue;
+				if (!tmp->present)
+					continue;
 
-			/* This device is sleeping and waiting to be serviced
-			 * earlier than any other device we checked thus far.
-			 */
-			if (tmp->sleep && (!sleep || time_after(sleep, tmp->sleep)))
-				sleep = tmp->sleep;
-		}
+				/* This device is sleeping and waiting to be serviced
+				 * earlier than any other device we checked thus far.
+				 */
+				if (tmp->sleep && (!sleep || time_after(sleep, tmp->sleep)))
+					sleep = tmp->sleep;
+			}
 
-		if (sleep) {
-			/*
-			 * Take a short snooze, and then wake up again.  Just
-			 * in case there are big differences in relative
-			 * throughputs.. don't want to hog the cpu too much.
-			 */
+			if (sleep) {
+				/*
+				 * Take a short snooze, and then wake up again.  Just
+				 * in case there are big differences in relative
+				 * throughputs.. don't want to hog the cpu too much.
+				 */
 
-			if (time_after(jiffies, sleep - WAIT_MIN_SLEEP))
-				sleep = jiffies + WAIT_MIN_SLEEP;
+				if (time_after(jiffies, sleep - WAIT_MIN_SLEEP))
+					sleep = jiffies + WAIT_MIN_SLEEP;
 #if 1
-			if (timer_pending(&channel->timer))
-				printk(KERN_ERR "%s: timer already active\n", __FUNCTION__);
+				if (timer_pending(&channel->timer))
+					printk(KERN_ERR "%s: timer already active\n", __FUNCTION__);
 #endif
-			set_bit(IDE_SLEEP, channel->active);
-			mod_timer(&channel->timer, sleep);
-
-			/*
-			 * We purposely leave us busy while sleeping becouse we
-			 * are prepared to handle the IRQ from it.
-			 *
-			 * FIXME: Make sure sleeping can't interferre with
-			 * operations of other devices on the same channel.
-			 */
-		} else {
-			/* FIXME: use queue plugging instead of active to block
-			 * upper layers from stomping on us */
-			/* Ugly, but how can we sleep for the lock otherwise?
-			 * */
+				set_bit(IDE_SLEEP, channel->active);
+				mod_timer(&channel->timer, sleep);
 
-			ide_release_lock(&ide_irq_lock);/* for atari only */
-			clear_bit(IDE_BUSY, channel->active);
+				/*
+				 * We purposely leave us busy while sleeping becouse we
+				 * are prepared to handle the IRQ from it.
+				 *
+				 * FIXME: Make sure sleeping can't interferre with
+				 * operations of other devices on the same channel.
+				 */
+			} else {
+				/* FIXME: use queue plugging instead of active to block
+				 * upper layers from stomping on us */
+				/* Ugly, but how can we sleep for the lock otherwise?
+				 * */
 
-			/* All requests are done.
-			 *
-			 * Disable IRQs from the last drive on this channel, to
-			 * make sure that it wan't throw stones at us when we
-			 * are not prepared to take them.
-			 */
+				ide_release_lock(&ide_irq_lock);/* for atari only */
+				clear_bit(IDE_BUSY, channel->active);
 
-			if (channel->drive && !channel->drive->using_tcq)
-				ata_irq_enable(channel->drive, 0);
-		}
+				/* All requests are done.
+				 *
+				 * Disable IRQs from the last drive on this channel, to
+				 * make sure that it wan't throw stones at us when we
+				 * are not prepared to take them.
+				 */
 
-		return;
-	}
+				if (channel->drive && !channel->drive->using_tcq)
+					ata_irq_enable(channel->drive, 0);
+			}
 
-	/* Remember the last drive we where acting on.
-	 */
-	ch = drive->channel;
-	ch->drive = drive;
+			return;
+		}
 
-	/* Feed commands to a drive until it barfs.
-	 */
-	do {
-		struct request *rq = NULL;
-		sector_t block;
+		/* Remember the last drive we where acting on.
+		 */
+		ch = drive->channel;
+		ch->drive = drive;
 
-		/* Abort early if we can't queue another command. for non tcq,
-		 * ata_can_queue is always 1 since we never get here unless the
-		 * drive is idle.
+		/* Feed commands to a drive until it barfs.
 		 */
+		do {
+			struct request *rq = NULL;
+			sector_t block;
 
-		if (!ata_can_queue(drive)) {
-			if (!ata_pending_commands(drive)) {
-				clear_bit(IDE_BUSY, ch->active);
-				if (drive->using_tcq)
-					ata_irq_enable(drive, 0);
-			}
-			break;
-		}
+			/* Abort early if we can't queue another command. for non tcq,
+			 * ata_can_queue is always 1 since we never get here unless the
+			 * drive is idle.
+			 */
 
-		drive->sleep = 0;
+			if (!ata_can_queue(drive)) {
+				if (!ata_pending_commands(drive)) {
+					clear_bit(IDE_BUSY, ch->active);
+					if (drive->using_tcq)
+						ata_irq_enable(drive, 0);
+				}
+				break;
+			}
 
-		if (test_bit(IDE_DMA, ch->active)) {
-			printk(KERN_ERR "%s: error: DMA in progress...\n", drive->name);
-			break;
-		}
+			drive->sleep = 0;
 
-		/* There's a small window between where the queue could be
-		 * replugged while we are in here when using tcq (in which case
-		 * the queue is probably empty anyways...), so check and leave
-		 * if appropriate. When not using tcq, this is still a severe
-		 * BUG!
-		 */
+			if (test_bit(IDE_DMA, ch->active)) {
+				printk(KERN_ERR "%s: error: DMA in progress...\n", drive->name);
+				break;
+			}
 
-		if (blk_queue_plugged(&drive->queue)) {
-			BUG_ON(!drive->using_tcq);
-			break;
-		}
+			/* There's a small window between where the queue could be
+			 * replugged while we are in here when using tcq (in which case
+			 * the queue is probably empty anyways...), so check and leave
+			 * if appropriate. When not using tcq, this is still a severe
+			 * BUG!
+			 */
 
-		if (!(rq = elv_next_request(&drive->queue))) {
-			if (!ata_pending_commands(drive)) {
-				clear_bit(IDE_BUSY, ch->active);
-				if (drive->using_tcq)
-					ata_irq_enable(drive, 0);
+			if (blk_queue_plugged(&drive->queue)) {
+				BUG_ON(!drive->using_tcq);
+				break;
 			}
-			drive->rq = NULL;
 
-			break;
-		}
+			if (!(rq = elv_next_request(&drive->queue))) {
+				if (!ata_pending_commands(drive)) {
+					clear_bit(IDE_BUSY, ch->active);
+					if (drive->using_tcq)
+						ata_irq_enable(drive, 0);
+				}
+				drive->rq = NULL;
 
-		/* If there are queued commands, we can't start a
-		 * non-fs request (really, a non-queuable command)
-		 * until the queue is empty.
-		 */
-		if (!(rq->flags & REQ_CMD) && ata_pending_commands(drive))
-			break;
+				break;
+			}
 
-		drive->rq = rq;
+			/* If there are queued commands, we can't start a
+			 * non-fs request (really, a non-queuable command)
+			 * until the queue is empty.
+			 */
+			if (!(rq->flags & REQ_CMD) && ata_pending_commands(drive))
+				break;
 
-		spin_unlock(ch->lock);
-		/* allow other IRQs while we start this request */
-		local_irq_enable();
+			drive->rq = rq;
 
-		/*
-		 * This initiates handling of a new I/O request.
-		 */
+			spin_unlock(ch->lock);
+			/* allow other IRQs while we start this request */
+			local_irq_enable();
 
-		BUG_ON(!(rq->flags & REQ_STARTED));
+			/*
+			 * This initiates handling of a new I/O request.
+			 */
+
+			BUG_ON(!(rq->flags & REQ_STARTED));
 
 #ifdef DEBUG
-		printk("%s: %s: current=0x%08lx\n", ch->name, __FUNCTION__, (unsigned long) rq);
+			printk("%s: %s: current=0x%08lx\n", ch->name, __FUNCTION__, (unsigned long) rq);
 #endif
 
-		/* bail early if we've exceeded max_failures */
-		if (drive->max_failures && (drive->failures > drive->max_failures))
-			goto kill_rq;
+			/* bail early if we've exceeded max_failures */
+			if (drive->max_failures && (drive->failures > drive->max_failures))
+				goto kill_rq;
 
-		block = rq->sector;
+			block = rq->sector;
 
-		/* Strange disk manager remap.
-		 */
-		if (rq->flags & REQ_CMD)
-			if (drive->type == ATA_DISK || drive->type == ATA_FLOPPY)
-				block += drive->sect0;
+			/* Strange disk manager remap.
+			 */
+			if (rq->flags & REQ_CMD)
+				if (drive->type == ATA_DISK || drive->type == ATA_FLOPPY)
+					block += drive->sect0;
 
-		/* Yecch - this will shift the entire interval, possibly killing some
-		 * innocent following sector.
-		 */
-		if (block == 0 && drive->remap_0_to_1 == 1)
-			block = 1;  /* redirect MBR access to EZ-Drive partn table */
+			/* Yecch - this will shift the entire interval, possibly killing some
+			 * innocent following sector.
+			 */
+			if (block == 0 && drive->remap_0_to_1 == 1)
+				block = 1;  /* redirect MBR access to EZ-Drive partn table */
 
-		ata_select(drive, 0);
-		ret = ata_status_poll(drive, drive->ready_stat, BUSY_STAT | DRQ_STAT,
-				WAIT_READY, rq);
+			ata_select(drive, 0);
+			ret = ata_status_poll(drive, drive->ready_stat, BUSY_STAT | DRQ_STAT,
+					WAIT_READY, rq);
 
-		if (ret != ATA_OP_READY) {
-			printk(KERN_ERR "%s: drive not ready for command\n", drive->name);
+			if (ret != ATA_OP_READY) {
+				printk(KERN_ERR "%s: drive not ready for command\n", drive->name);
 
-			goto kill_rq;
-		}
+				goto kill_rq;
+			}
 
-		if (!ata_ops(drive)) {
-			printk(KERN_WARNING "%s: device type %d not supported\n",
-					drive->name, drive->type);
-			goto kill_rq;
-		}
+			if (!ata_ops(drive)) {
+				printk(KERN_WARNING "%s: device type %d not supported\n",
+						drive->name, drive->type);
+				goto kill_rq;
+			}
 
-		/* The normal way of execution is to pass and execute the request
-		 * handler down to the device type driver.
-		 */
+			/* The normal way of execution is to pass and execute the request
+			 * handler down to the device type driver.
+			 */
 
-		if (ata_ops(drive)->do_request) {
-			ret = ata_ops(drive)->do_request(drive, rq, block);
-		} else {
+			if (ata_ops(drive)->do_request) {
+				ret = ata_ops(drive)->do_request(drive, rq, block);
+			} else {
 kill_rq:
-			if (ata_ops(drive) && ata_ops(drive)->end_request)
-				ata_ops(drive)->end_request(drive, rq, 0);
-			else
-				ata_end_request(drive, rq, 0, 0);
-			ret = ATA_OP_FINISHED;
-
-		}
-		spin_lock_irq(ch->lock);
-
-		/* continue if command started, so we are busy */
-	} while (ret != ATA_OP_CONTINUES);
-	/* make sure the BUSY bit is set */
-	/* FIXME: perhaps there is some place where we miss to set it? */
-//		set_bit(IDE_BUSY, ch->active);
-}
+				if (ata_ops(drive) && ata_ops(drive)->end_request)
+					ata_ops(drive)->end_request(drive, rq, 0);
+				else
+					ata_end_request(drive, rq, 0, 0);
+				ret = ATA_OP_FINISHED;
 
-void do_ide_request(request_queue_t *q)
-{
-	struct ata_channel *ch = q->queuedata;
+			}
+			spin_lock_irq(ch->lock);
 
-	while (!test_and_set_bit(IDE_BUSY, ch->active)) {
-		do_request(ch);
+			/* continue if command started, so we are busy */
+		} while (ret != ATA_OP_CONTINUES);
+		/* make sure the BUSY bit is set */
+		/* FIXME: perhaps there is some place where we miss to set it? */
+		//		set_bit(IDE_BUSY, ch->active);
 	}
 }
 
 /*
  * This is our timeout function for all drive operations.  But note that it can
  * also be invoked as a result of a "sleep" operation triggered by the
- * mod_timer() call in do_request.
+ * mod_timer() call in do_ide_request.
  *
  * FIXME: This should take a drive context instead of a channel.
  * FIXME: This should not explicitly reenter the request handling engine.

@@ -893,7 +886,8 @@ void ide_timer_expiry(unsigned long data
 
 		if (ret == ATA_OP_FINISHED) {
 			/* Reenter the request handling engine. */
-			do_request(ch);
+			clear_bit(IDE_BUSY, ch->active);
+			do_ide_request(&drive->queue);
 		}
 	}
 	spin_unlock_irqrestore(ch->lock, flags);

@@ -1050,9 +1044,10 @@ void ata_irq_request(int irq, void *data
 		 * another interrupt.
 		 */
 
-		if (!ch->handler)
-			do_request(ch);
-		else
+		if (!ch->handler) {
+			clear_bit(IDE_BUSY, ch->active);
+			do_ide_request(&drive->queue);
+		} else
 			printk("%s: %s: huh? expected NULL handler on exit\n",
 					drive->name, __FUNCTION__);
 	}