2.6.17-rc4 + libata-pata Oops while reloading pata_amd module

2.6.17-rc4 + libata-pata Oops while reloading pata_amd module

[Erik Mouw]

Hi Alan,

I got this Oops while reloading the pata_amd module:

pata_amd 0000:00:07.1: version 0.1.7
ata3: PATA max UDMA/100 cmd 0x1F0 ctl 0x3F6 bmdma 0xD800 irq 14
setup_irq: irq handler mismatch
 <c013cb24> setup_irq+0x114/0x130   <e088a930> ata_interrupt+0x0/0x130 [libata]
 <c013cd14> request_irq+0x84/0xb0   <e088b192> ata_device_add+0x182/0x250 [libata]
 <e088a930> ata_interrupt+0x0/0x130 [libata]   <e088ecf1> ata_pci_init_one+0x181/0x3f0 [libata]
 <c02b53b5> _spin_unlock_irqrestore+0x5/0x10   <c01d873a> pci_bus_write_config_byte+0x5a/0x70
 <e0839a8d> amd_init_one+0xdd/0x160 [pata_amd]   <c01dd2e9> pci_call_probe+0x19/0x20
 <c01dd34e> __pci_device_probe+0x5e/0x70   <c01dd38f> pci_device_probe+0x2f/0x50
 <c0220ab7> driver_probe_device+0xb7/0xe0   <c02b340a> klist_dec_and_del+0x1a/0x20
 <c0220b70> __driver_attach+0x0/0x90   <c0220be1> __driver_attach+0x71/0x90
 <c021fe89> bus_for_each_dev+0x69/0x80   <c0220c26> driver_attach+0x26/0x30
 <c0220b70> __driver_attach+0x0/0x90   <c02203e3> bus_add_driver+0x83/0xc0
 <c01dd62d> __pci_register_driver+0x4d/0x70   <e0870017> amd_init+0x17/0x1b [pata_amd]
 <c013a1e0> sys_init_module+0x120/0x1b0   <c0102f27> syscall_call+0x7/0xb
BUG: unable to handle kernel NULL pointer dereference at virtual address 00000000
 printing eip:
c0221cd1
*pde = 00000000
Oops: 0000 [#1]
SMP
Modules linked in: pata_amd nbd ohci_hcd usbcore hw_random i2c_amd756 i2c_core libata scsi_mod amd_k7_agp agpgart 8250 serial_core md_mod
CPU:    0
EIP:    0060:[<c0221cd1>]    Not tainted VLI
EFLAGS: 00010206   (2.6.17-rc4-dirty #2)
EIP is at make_class_name+0x31/0xb0
eax: 00000000   ebx: ffffffff   ecx: ffffffff   edx: 00000009
esi: 00000000   edi: 00000000   ebp: 00000000   esp: c157fd68
ds: 007b   es: 007b   ss: 0068
Process modprobe (pid: 1867, threadinfo=c157f000 task=dfd6dab0)
Stack: <0>0000000e 00287e09 df0ba0e0 df0ba228 df0ba220 df0ba0e0 e08bbb20 c02221cf
       df0ba220 dfd6dbbc 00000000 00000000 df0ba220 df0ba000 df0ba030 dffb9000
       c0222233 df0ba220 df0ba0e0 e089d467 df0ba220 00000004 df0ba2c0 df0ed760
Call Trace:
 <c02221cf> class_device_del+0xff/0x150   <c0222233> class_device_unregister+0x13/0x30
 <e089d467> scsi_remove_host+0xb7/0x110 [scsi_mod]   <e088adee> ata_host_remove+0x2e/0x30 [libata]
 <e088b234> ata_device_add+0x224/0x250 [libata]   <e088ecf1> ata_pci_init_one+0x181/0x3f0 [libata]
 <c02b53b5> _spin_unlock_irqrestore+0x5/0x10   <c01d873a> pci_bus_write_config_byte+0x5a/0x70
 <e0839a8d> amd_init_one+0xdd/0x160 [pata_amd]   <c01dd2e9> pci_call_probe+0x19/0x20
 <c01dd34e> __pci_device_probe+0x5e/0x70   <c01dd38f> pci_device_probe+0x2f/0x50
 <c0220ab7> driver_probe_device+0xb7/0xe0   <c02b340a> klist_dec_and_del+0x1a/0x20
 <c0220b70> __driver_attach+0x0/0x90   <c0220be1> __driver_attach+0x71/0x90
 <c021fe89> bus_for_each_dev+0x69/0x80   <c0220c26> driver_attach+0x26/0x30
 <c0220b70> __driver_attach+0x0/0x90   <c02203e3> bus_add_driver+0x83/0xc0
 <c01dd62d> __pci_register_driver+0x4d/0x70   <e0870017> amd_init+0x17/0x1b [pata_amd]
 <c013a1e0> sys_init_module+0x120/0x1b0   <c0102f27> syscall_call+0x7/0xb
Code: bb ff ff ff ff 83 ec 0c 89 d9 8b 54 24 20 8b 42 48 8b 10 89 e8 89 d7 f2 ae f7 d1 49 8b 44 24 20 89 ca 89 d9 8b 70 08 89 e8 89 f7 <f2> ae f7 d1 49 c7 44 24 04 d0 00 00 00 8d 44 11 02 89 04 24 e8
EIP: [<c0221cd1>] make_class_name+0x31/0xb0 SS:ESP 0068:c157fd68

This is with 2.6.17-rc4 with patch-2.6.17-rc4-ide1.gz applied on top of
it. I could also recreate it with today's -git kernel with your patch
applied on top of it (applies cleanly with some minor fuzz).

Here's the lspci output:

00:00.0 Host bridge: Advanced Micro Devices [AMD]: Unknown device 700c (rev 11)
        Flags: bus master, 66Mhz, medium devsel, latency 32
        Memory at fc000000 (32-bit, prefetchable) [size=32M]
        Memory at fb800000 (32-bit, prefetchable) [size=4K]
        I/O ports at e800 [disabled] [size=4]
        Capabilities: [a0] AGP version 2.0
00: 22 10 0c 70 06 00 30 22 11 00 00 06 00 20 00 00
10: 08 00 00 fc 08 00 80 fb 01 e8 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 a0 00 00 00 00 00 00 00 00 00 00 00
40: 00 00 00 00 18 1b 00 00 00 00 00 00 27 00 00 00
50: 30 77 77 00 4a 8c 01 96 00 00 22 02 00 00 00 00
60: bd 0c a1 85 1b 2e e2 5e bd 8c b1 00 1b 25 d2 5f
70: 00 06 04 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 05 00 91 10 83 00 71 0f f0 02 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 20
a0: 02 00 20 00 03 02 00 0f 00 00 00 00 01 00 01 00
b0: 00 00 00 00 48 00 01 2a 0f ff 0f c5 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 85 0f 00 00 85 0f 00 10
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:01.0 PCI bridge: Advanced Micro Devices [AMD]: Unknown device 700d (prog-if 00 [Normal decode])
        Flags: bus master, 66Mhz, medium devsel, latency 64
        Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
        Memory behind bridge: f4000000-fb4fffff
        Prefetchable memory behind bridge: fb700000-fb7fffff
00: 22 10 0d 70 07 00 20 02 00 00 04 06 00 40 01 00
10: 00 00 00 00 00 00 00 00 00 01 01 00 f1 01 20 22
20: 00 f4 40 fb 70 fb 70 fb 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 08 00
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:07.0 ISA bridge: Advanced Micro Devices [AMD]: Unknown device 7440 (rev 05)
        Subsystem: Asustek Computer, Inc.: Unknown device 8044
        Flags: bus master, 66Mhz, medium devsel, latency 0
00: 22 10 40 74 0f 00 20 02 05 00 01 06 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 44 80
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
40: 00 20 07 00 01 00 00 00 0b ff 00 81 00 04 00 00
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 43 10 44 80 00 de 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:07.1 IDE interface: Advanced Micro Devices [AMD]: Unknown device 7441 (rev 04) (prog-if 8a [Master SecP PriP])
        Subsystem: Advanced Micro Devices [AMD]: Unknown device 7441
        Flags: bus master, medium devsel, latency 32
        I/O ports at d800 [size=16]
00: 22 10 41 74 05 00 00 02 04 8a 01 01 00 20 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 01 d8 00 00 00 00 00 00 00 00 00 00 22 10 41 74
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
40: 43 f4 00 00 00 00 00 00 a8 a8 a8 a8 ff 00 ff ff
50: 03 03 03 03 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 22 10 41 74 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:07.3 Bridge: Advanced Micro Devices [AMD]: Unknown device 7443 (rev 03)
        Subsystem: Asustek Computer, Inc.: Unknown device 8044
        Flags: medium devsel
00: 22 10 43 74 00 00 80 02 03 00 80 06 00 20 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 44 80
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
40: 80 b1 09 07 00 00 00 00 a8 00 50 00 00 00 00 00
50: 01 80 00 00 0f 00 0b a0 01 e4 00 00 00 00 00 00
60: 00 00 80 06 1f 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 44 80
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 60 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 73 84 62 00 00 00 00 00 00 00 00 00 00 00 00 00

00:08.0 Ethernet controller: Intel Corp.: Unknown device 100f (rev 01)
        Subsystem: Intel Corp.: Unknown device 1001
        Flags: bus master, 66Mhz, medium devsel, latency 32, IRQ 16
        Memory at f3800000 (64-bit, non-prefetchable) [size=128K]
        Memory at f3000000 (64-bit, non-prefetchable) [size=256K]
        I/O ports at d400 [size=64]
        Expansion ROM at 30000000 [disabled] [size=256K]
        Capabilities: [dc] Power Management version 2
        Capabilities: [e4] #07 [0002]
        Capabilities: [f0] Message Signalled Interrupts: 64bit+ Queue=0/0 Enable-
00: 86 80 0f 10 17 00 30 02 01 00 00 02 08 20 00 00
10: 04 00 80 f3 00 00 00 00 04 00 00 f3 00 00 00 00
20: 01 d4 00 00 00 00 00 00 00 00 00 00 86 80 01 10
30: 00 00 00 00 dc 00 00 00 00 00 00 00 0b 01 ff 00
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 01 e4 22 00
e0: 00 00 00 00 07 f0 02 00 00 00 43 04 00 00 00 00
f0: 05 00 80 00 00 00 00 00 00 00 00 00 00 00 00 00

00:10.0 PCI bridge: Advanced Micro Devices [AMD]: Unknown device 7448 (rev 05) (prog-if 00 [Normal decode])
        Flags: bus master, 66Mhz, medium devsel, latency 32
        Bus: primary=00, secondary=02, subordinate=02, sec-latency=32
        Memory behind bridge: f2800000-f2ffffff
        Prefetchable memory behind bridge: fb500000-fb5fffff
00: 22 10 48 74 17 00 20 22 05 00 04 06 00 20 01 00
10: 00 00 00 00 00 00 00 00 00 02 02 20 f0 00 00 22
20: 80 f2 f0 f2 50 fb 50 fb 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 ff 00 04 00
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

01:05.0 VGA compatible controller: S3 Inc. ViRGE/GX2 (rev 04) (prog-if 00 [VGA])
        Subsystem: Diamond Multimedia Systems Stealth 3D 4000
        Flags: bus master, medium devsel, latency 64, IRQ 16
        Memory at f4000000 (32-bit, non-prefetchable) [size=64M]
        Expansion ROM at fb7f0000 [disabled] [size=64K]
00: 33 53 10 8a 07 00 00 02 04 00 00 03 00 40 00 00
10: 00 00 00 f4 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 92 10 10 8a
30: 00 00 7f fb 00 00 00 00 00 00 00 00 0b 01 04 ff
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

02:00.0 USB Controller: Advanced Micro Devices [AMD]: Unknown device 7449 (rev 07) (prog-if 10 [OHCI])
        Subsystem: Asustek Computer, Inc.: Unknown device 8044
        Flags: bus master, medium devsel, latency 32, IRQ 19
        Memory at f2800000 (32-bit, non-prefetchable) [size=4K]
00: 22 10 49 74 17 00 80 02 07 10 03 0c 08 20 00 00
10: 00 00 80 f2 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 44 80
30: 00 00 00 00 00 00 00 00 00 00 00 00 0a 04 00 50
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 43 10 44 80 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Output from lsmod:

Module                  Size  Used by
pata_amd               10151  1
nbd                    19936  0
ohci_hcd               20484  0
usbcore               130432  2 ohci_hcd
hw_random               5592  0
i2c_amd756              6020  0
i2c_core               18688  1 i2c_amd756
libata                 69324  1 pata_amd
scsi_mod              137864  1 libata
amd_k7_agp              7372  1
agpgart                32136  1 amd_k7_agp
8250                   23588  2
serial_core            19712  1 8250
md_mod                 74772  0

The machine is an Asus A7M266-D dual Athlon board with only a single
CPU installed running Debian stable on NFS root.

How to recreate:

- Fully modular scsi and libata, root on NFS
- rmmod pata_amd
- modprobe pata_amd

Works with any drive I've tried.


Erik

PS: note that I'm not at the office till monday, so can't really test
new patches right now.

-- 
+-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands

Re: 2.6.17-rc4 + libata-pata Oops while reloading pata_amd module

[Albert Lee]

Erik Mouw wrote:
> Hi Alan,
> 
> I got this Oops while reloading the pata_amd module:
> 
<snip>
> 
> The machine is an Asus A7M266-D dual Athlon board with only a single
> CPU installed running Debian stable on NFS root.
> 
> How to recreate:
> 
> - Fully modular scsi and libata, root on NFS
> - rmmod pata_amd
> - modprobe pata_amd
> 
> Works with any drive I've tried.
> 
> 

Hi Erik,

Unicorn Chang (on cc list) is working on a patch for this problem.
Will submit the patch later.

--
albert

[PATCH/RFC 0/2] libata: legacy mode fixes

[Albert Lee]

Dear all,

Description:
This is the revised patch by Unicorn and I for legacy mode fix.

patch 1/2: make both legacy ports share one host_set.
patch 2/2: remove the unused ap->hard_port_no.

Patch against the upstream branch of libata-dev tree 
(957d2df1801865eb1e63864bc63b970aa9c460ba).

For your review, thanks.

Unicorn Chang
Albert Lee

[PATCH/RFC 1/2] libata: make both legacy ports share one host_set

[Albert Lee]

Patch 1/2: 
- make both legacy ports share one host_set
- use signed irq (-1) if variable not used
- host_set->irq is preserved for shared irq case(PCI native mode).
- ap->ioaddr.irq is added for per-port non-shared irq case(legacy mode).
- ATA_FLAG_LEGACY added to flag the legacy ports

Signed-off-by: Unicorn Chang <uchang@tw.ibm.com>
Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
The host_set is shared between the 2 legacy ports
for the ATA_HOST_SIMPLEX to work.
The per-port irq is also freed for the pata_amd unloading to load.
Briefly tested on x86 box.

For your review, thanks.

--- upstream0/include/linux/libata.h	2006-05-25 10:45:36.000000000 +0800
+++ 1_legacy_irq/include/linux/libata.h	2006-05-26 09:19:40.000000000 +0800
@@ -149,8 +149,9 @@ enum {
 	ATA_FLAG_NO_ATAPI	= (1 << 6), /* No ATAPI support */
 	ATA_FLAG_PIO_DMA	= (1 << 7), /* PIO cmds via DMA */
 	ATA_FLAG_PIO_LBA48	= (1 << 8), /* Host DMA engine is LBA28 only */
-	ATA_FLAG_PIO_POLLING	= (1 << 10), /* use polling PIO if LLD
+	ATA_FLAG_PIO_POLLING	= (1 << 9), /* use polling PIO if LLD
 					      * doesn't handle PIO interrupts */
+	ATA_FLAG_LEGACY		= (1 << 10), /* port is legacy */
 	ATA_FLAG_NCQ		= (1 << 11), /* host supports NCQ */
 
 	ATA_FLAG_DEBUGMSG	= (1 << 14),
@@ -301,6 +302,7 @@ struct ata_ioports {
 	unsigned long		ctl_addr;
 	unsigned long		bmdma_addr;
 	unsigned long		scr_addr;
+	long			irq;
 };
 
 struct ata_probe_ent {
@@ -310,12 +312,11 @@ struct ata_probe_ent {
 	struct scsi_host_template *sht;
 	struct ata_ioports	port[ATA_MAX_PORTS];
 	unsigned int		n_ports;
-	unsigned int		hard_port_no;
 	unsigned int		pio_mask;
 	unsigned int		mwdma_mask;
 	unsigned int		udma_mask;
 	unsigned int		legacy_mode;
-	unsigned long		irq;
+	long			irq;
 	unsigned int		irq_flags;
 	unsigned long		host_flags;
 	unsigned long		host_set_flags;
@@ -326,7 +327,7 @@ struct ata_probe_ent {
 struct ata_host_set {
 	spinlock_t		lock;
 	struct device 		*dev;
-	unsigned long		irq;
+	long			irq;
 	void __iomem		*mmio_base;
 	unsigned int		n_ports;
 	void			*private_data;
--- upstream0/drivers/scsi/libata-core.c	2006-05-25 10:45:21.000000000 +0800
+++ 1_legacy_irq/drivers/scsi/libata-core.c	2006-05-26 09:19:40.000000000 +0800
@@ -4776,11 +4776,36 @@ irqreturn_t ata_interrupt (int irq, void
 	/* TODO: make _irqsave conditional on x86 PCI IDE legacy mode */
 	spin_lock_irqsave(&host_set->lock, flags);
 
+	if (irq != host_set->irq) {
+		/* per-port irq */
+		for (i = 0; i < host_set->n_ports; i++) {
+			struct ata_port *ap = host_set->ports[i];
+
+			if (ap && (ap->flags & ATA_FLAG_LEGACY) &&
+			    (ap->ioaddr.irq == irq) &&
+			    !(ap->flags & ATA_FLAG_DISABLED)) {
+				struct ata_queued_cmd *qc;
+
+				qc = ata_qc_from_tag(ap, ap->active_tag);
+				if (qc &&
+				    (!(qc->tf.flags & ATA_TFLAG_POLLING)) &&
+				    (qc->flags & ATA_QCFLAG_ACTIVE))
+					handled |= ata_host_intr(ap, qc);
+				break;
+			}
+		}
+
+		spin_unlock_irqrestore(&host_set->lock, flags);
+
+		return IRQ_RETVAL(handled);
+	}
+
+	/* shared irq */
 	for (i = 0; i < host_set->n_ports; i++) {
 		struct ata_port *ap;
 
 		ap = host_set->ports[i];
-		if (ap &&
+		if (ap && !(ap->flags & ATA_FLAG_LEGACY) &&
 		    !(ap->flags & ATA_FLAG_DISABLED)) {
 			struct ata_queued_cmd *qc;
 
@@ -5148,8 +5173,7 @@ static void ata_host_init(struct ata_por
 	ap->host_set = host_set;
 	ap->dev = ent->dev;
 	ap->port_no = port_no;
-	ap->hard_port_no =
-		ent->legacy_mode ? ent->hard_port_no : port_no;
+	ap->hard_port_no = port_no;
 	ap->pio_mask = ent->pio_mask;
 	ap->mwdma_mask = ent->mwdma_mask;
 	ap->udma_mask = ent->udma_mask;
@@ -5282,6 +5306,7 @@ int ata_device_add(const struct ata_prob
 	for (i = 0; i < ent->n_ports; i++) {
 		struct ata_port *ap;
 		unsigned long xfer_mode_mask;
+		unsigned long irq;
 
 		ap = ata_host_add(ent, host_set, i);
 		if (!ap)
@@ -5292,6 +5317,12 @@ int ata_device_add(const struct ata_prob
 				(ap->mwdma_mask << ATA_SHIFT_MWDMA) |
 				(ap->pio_mask << ATA_SHIFT_PIO);
 
+		if (ent->legacy_mode) {
+			irq = ap->ioaddr.irq;
+			ap->flags |= ATA_FLAG_LEGACY;
+		} else
+			irq = ent->irq;
+
 		/* print per-port info to dmesg */
 		ata_port_printk(ap, KERN_INFO, "%cATA max %s cmd 0x%lX "
 				"ctl 0x%lX bmdma 0x%lX irq %lu\n",
@@ -5300,21 +5331,28 @@ int ata_device_add(const struct ata_prob
 				ap->ioaddr.cmd_addr,
 				ap->ioaddr.ctl_addr,
 				ap->ioaddr.bmdma_addr,
-				ent->irq);
+				irq);
 
 		ata_chk_status(ap);
 		host_set->ops->irq_clear(ap);
 		ata_eh_freeze_port(ap);	/* freeze port before requesting IRQ */
+
+		if (ap->flags & ATA_FLAG_LEGACY)
+			/* obtain legacy per-port irq */
+			if (request_irq(irq, ent->port_ops->irq_handler, 0,
+				DRV_NAME, host_set))
+				goto err_out;
 		count++;
 	}
 
 	if (!count)
 		goto err_free_ret;
 
-	/* obtain irq, that is shared between channels */
-	if (request_irq(ent->irq, ent->port_ops->irq_handler, ent->irq_flags,
-			DRV_NAME, host_set))
-		goto err_out;
+	if (!ent->legacy_mode)
+		/* obtain irq, that is shared between channels */
+		if (request_irq(ent->irq, ent->port_ops->irq_handler,
+				ent->irq_flags, DRV_NAME, host_set))
+			goto err_out;
 
 	/* perform each probe synchronously */
 	DPRINTK("probe begin\n");
@@ -5387,31 +5425,35 @@ void ata_host_set_remove(struct ata_host
 {
 	struct ata_port *ap;
 	unsigned int i;
+	int native = 0;
 
 	for (i = 0; i < host_set->n_ports; i++) {
 		ap = host_set->ports[i];
 		scsi_remove_host(ap->host);
 	}
 
-	free_irq(host_set->irq, host_set);
-
 	for (i = 0; i < host_set->n_ports; i++) {
 		ap = host_set->ports[i];
 
 		ata_scsi_release(ap->host);
 
-		if ((ap->flags & ATA_FLAG_NO_LEGACY) == 0) {
+		if (ap->flags & ATA_FLAG_LEGACY) {
 			struct ata_ioports *ioaddr = &ap->ioaddr;
 
-			if (ioaddr->cmd_addr == 0x1f0)
-				release_region(0x1f0, 8);
-			else if (ioaddr->cmd_addr == 0x170)
-				release_region(0x170, 8);
-		}
+			WARN_ON(!(ioaddr->cmd_addr == 0x1f0 ||
+				  ioaddr->cmd_addr == 0x170));
+
+			release_region(ioaddr->cmd_addr, 8);
+			free_irq(ioaddr->irq, host_set);
+		} else
+			native++;
 
 		scsi_host_put(ap->host);
 	}
 
+	if (native)
+		free_irq(host_set->irq, host_set);
+
 	if (host_set->ops->host_stop)
 		host_set->ops->host_stop(host_set);
 
--- upstream0/drivers/scsi/libata-bmdma.c	2006-05-25 10:45:21.000000000 +0800
+++ 1_legacy_irq/drivers/scsi/libata-bmdma.c	2006-05-26 09:19:40.000000000 +0800
@@ -879,7 +879,7 @@ ata_pci_init_native_mode(struct pci_dev 
 		if (bmdma) {
 			bmdma += 8;
 			if(inb(bmdma + 2) & 0x80)
-			probe_ent->host_set_flags |= ATA_HOST_SIMPLEX;
+				probe_ent->host_set_flags |= ATA_HOST_SIMPLEX;
 			probe_ent->port[p].bmdma_addr = bmdma;
 		}
 		ata_std_ports(&probe_ent->port[p]);
@@ -891,46 +891,67 @@ ata_pci_init_native_mode(struct pci_dev 
 }
 
 
-static struct ata_probe_ent *ata_pci_init_legacy_port(struct pci_dev *pdev,
-				struct ata_port_info *port, int port_num)
+/**
+ *	ata_pci_init_legacy_mode - Initialize legacy-mode driver
+ *	@pdev:  pci device to be initialized
+ *	@port:  array[2] of pointers to port info structures.
+ *	@ports: bitmap of ports present
+ *
+ *	Utility function which allocates and initializes an
+ *	ata_probe_ent structure for a standard dual-port
+ *	PIO-based IDE controller.  The returned ata_probe_ent
+ *	structure can be passed to ata_device_add().  The returned
+ *	ata_probe_ent structure should then be freed with kfree().
+ */
+static struct ata_probe_ent *ata_pci_init_legacy_mode(struct pci_dev *pdev,
+				struct ata_port_info **port, int ports)
 {
-	struct ata_probe_ent *probe_ent;
+	struct ata_probe_ent *probe_ent =
+		ata_probe_ent_alloc(pci_dev_to_dev(pdev), port[0]);
+	int p = 0;
 	unsigned long bmdma;
 
-	probe_ent = ata_probe_ent_alloc(pci_dev_to_dev(pdev), port);
 	if (!probe_ent)
 		return NULL;
 
 	probe_ent->legacy_mode = 1;
-	probe_ent->n_ports = 1;
-	probe_ent->hard_port_no = port_num;
-	probe_ent->private_data = port->private_data;
-
-	switch(port_num)
-	{
-		case 0:
-			probe_ent->irq = 14;
-			probe_ent->port[0].cmd_addr = 0x1f0;
-			probe_ent->port[0].altstatus_addr =
-			probe_ent->port[0].ctl_addr = 0x3f6;
-			break;
-		case 1:
-			probe_ent->irq = 15;
-			probe_ent->port[0].cmd_addr = 0x170;
-			probe_ent->port[0].altstatus_addr =
-			probe_ent->port[0].ctl_addr = 0x376;
-			break;
+	probe_ent->irq = -1;
+	probe_ent->private_data = port[0]->private_data;
+
+	if (ports & ATA_PORT_PRIMARY) {
+		probe_ent->port[p].irq = 14;
+		probe_ent->port[p].cmd_addr = 0x1f0;
+		probe_ent->port[p].altstatus_addr =
+			probe_ent->port[p].ctl_addr = 0x3f6;
+
+		bmdma = pci_resource_start(pdev, 4);
+		if (bmdma) {
+			probe_ent->port[p].bmdma_addr = bmdma;
+			if (inb(bmdma + 2) & 0x80)
+				probe_ent->host_set_flags |= ATA_HOST_SIMPLEX;
+		}
+		ata_std_ports(&probe_ent->port[p]);
+		p++;
 	}
 
-	bmdma = pci_resource_start(pdev, 4);
-	if (bmdma != 0) {
-		bmdma += 8 * port_num;
-		probe_ent->port[0].bmdma_addr = bmdma;
-		if (inb(bmdma + 2) & 0x80)
-			probe_ent->host_set_flags |= ATA_HOST_SIMPLEX;
+	if (ports & ATA_PORT_SECONDARY) {
+		probe_ent->port[p].irq = 15;
+		probe_ent->port[p].cmd_addr = 0x170;
+		probe_ent->port[p].altstatus_addr =
+			probe_ent->port[p].ctl_addr = 0x376;
+
+		bmdma = pci_resource_start(pdev, 4);
+		if (bmdma) {
+			bmdma += 8;
+			probe_ent->port[p].bmdma_addr = bmdma;
+			if(inb(bmdma + 2) & 0x80)
+				probe_ent->host_set_flags |= ATA_HOST_SIMPLEX;
+		}
+		ata_std_ports(&probe_ent->port[p]);
+		p++;
 	}
-	ata_std_ports(&probe_ent->port[0]);
 
+	probe_ent->n_ports = p;
 	return probe_ent;
 }
 
@@ -959,10 +980,10 @@ static struct ata_probe_ent *ata_pci_ini
 int ata_pci_init_one (struct pci_dev *pdev, struct ata_port_info **port_info,
 		      unsigned int n_ports)
 {
-	struct ata_probe_ent *probe_ent = NULL, *probe_ent2 = NULL;
+	struct ata_probe_ent *probe_ent = NULL;
 	struct ata_port_info *port[2];
 	u8 tmp8, mask;
-	unsigned int legacy_mode = 0;
+	int legacy_mode = 0;
 	int disable_dev_on_err = 1;
 	int rc;
 
@@ -1054,17 +1075,14 @@ int ata_pci_init_one (struct pci_dev *pd
 		goto err_out_regions;
 
 	if (legacy_mode) {
-		if (legacy_mode & (1 << 0))
-			probe_ent = ata_pci_init_legacy_port(pdev, port[0], 0);
-		if (legacy_mode & (1 << 1))
-			probe_ent2 = ata_pci_init_legacy_port(pdev, port[1], 1);
+		probe_ent = ata_pci_init_legacy_mode(pdev, port, legacy_mode);
 	} else {
 		if (n_ports == 2)
 			probe_ent = ata_pci_init_native_mode(pdev, port, ATA_PORT_PRIMARY | ATA_PORT_SECONDARY);
 		else
 			probe_ent = ata_pci_init_native_mode(pdev, port, ATA_PORT_PRIMARY);
 	}
-	if (!probe_ent && !probe_ent2) {
+	if (!probe_ent) {
 		rc = -ENOMEM;
 		goto err_out_regions;
 	}
@@ -1072,16 +1090,9 @@ int ata_pci_init_one (struct pci_dev *pd
 	pci_set_master(pdev);
 
 	/* FIXME: check ata_device_add return */
-	if (legacy_mode) {
-		if (legacy_mode & (1 << 0))
-			ata_device_add(probe_ent);
-		if (legacy_mode & (1 << 1))
-			ata_device_add(probe_ent2);
-	} else
-		ata_device_add(probe_ent);
+	ata_device_add(probe_ent);
 
 	kfree(probe_ent);
-	kfree(probe_ent2);
 
 	return 0;
 

[PATCH/RFC 2/2] libata: replace ap->hard_port_no with ap->port_no

[Albert Lee]

Patch 2/2:
replace ap->hard_port_no with ap->port_no.

Signed-off-by: Unicorn Chang <uchang@tw.ibm.com>
Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---

Since now ap->port_no == ap->hard_port_no.
Replace/remove ap->hard_port_no.

--- 1_legacy_irq/include/linux/libata.h	2006-05-26 09:19:40.000000000 +0800
+++ 2_remove_hardport/include/linux/libata.h	2006-05-26 09:20:01.000000000 +0800
@@ -449,7 +449,6 @@ struct ata_port {
 	unsigned long		flags;	/* ATA_FLAG_xxx */
 	unsigned int		id;	/* unique id req'd by scsi midlyr */
 	unsigned int		port_no; /* unique port #; from zero */
-	unsigned int		hard_port_no;	/* hardware port #; from zero */
 
 	struct ata_prd		*prd;	 /* our SG list */
 	dma_addr_t		prd_dma; /* and its DMA mapping */
--- 1_legacy_irq/drivers/scsi/libata-core.c	2006-05-26 09:19:40.000000000 +0800
+++ 2_remove_hardport/drivers/scsi/libata-core.c	2006-05-26 09:20:01.000000000 +0800
@@ -5173,7 +5173,6 @@ static void ata_host_init(struct ata_por
 	ap->host_set = host_set;
 	ap->dev = ent->dev;
 	ap->port_no = port_no;
-	ap->hard_port_no = port_no;
 	ap->pio_mask = ent->pio_mask;
 	ap->mwdma_mask = ent->mwdma_mask;
 	ap->udma_mask = ent->udma_mask;
--- 0_piix/drivers/scsi/ata_piix.c	2006-05-26 13:23:55.000000000 +0800
+++ 2_remove_hardport/drivers/scsi/ata_piix.c	2006-05-26 13:26:38.000000000 +0800
@@ -451,7 +451,7 @@ static void piix_pata_cbl_detect(struct 
 		goto cbl40;
 
 	/* check BIOS cable detect results */
-	mask = ap->hard_port_no == 0 ? PIIX_80C_PRI : PIIX_80C_SEC;
+	mask = ap->port_no == 0 ? PIIX_80C_PRI : PIIX_80C_SEC;
 	pci_read_config_byte(pdev, PIIX_IOCFG, &tmp);
 	if ((tmp & mask) == 0)
 		goto cbl40;
@@ -493,7 +493,7 @@ static int piix_pata_probe_reset(struct 
 {
 	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
 
-	if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->hard_port_no])) {
+	if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->port_no])) {
 		ata_port_printk(ap, KERN_INFO, "port disabled. ignoring.\n");
 		return 0;
 	}
@@ -521,7 +521,7 @@ static unsigned int piix_sata_probe (str
 {
 	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
 	const unsigned int *map = ap->host_set->private_data;
-	int base = 2 * ap->hard_port_no;
+	int base = 2 * ap->port_no;
 	unsigned int present_mask = 0;
 	int port, i;
 	u8 pcs;
@@ -600,7 +600,7 @@ static void piix_set_piomode (struct ata
 	unsigned int pio	= adev->pio_mode - XFER_PIO_0;
 	struct pci_dev *dev	= to_pci_dev(ap->host_set->dev);
 	unsigned int is_slave	= (adev->devno != 0);
-	unsigned int master_port= ap->hard_port_no ? 0x42 : 0x40;
+	unsigned int master_port= ap->port_no ? 0x42 : 0x40;
 	unsigned int slave_port	= 0x44;
 	u16 master_data;
 	u8 slave_data;
@@ -618,10 +618,10 @@ static void piix_set_piomode (struct ata
 		/* enable PPE, IE and TIME */
 		master_data |= 0x0070;
 		pci_read_config_byte(dev, slave_port, &slave_data);
-		slave_data &= (ap->hard_port_no ? 0x0f : 0xf0);
+		slave_data &= (ap->port_no ? 0x0f : 0xf0);
 		slave_data |=
 			(timings[pio][0] << 2) |
-			(timings[pio][1] << (ap->hard_port_no ? 4 : 0));
+			(timings[pio][1] << (ap->port_no ? 4 : 0));
 	} else {
 		master_data &= 0xccf8;
 		/* enable PPE, IE and TIME */
@@ -651,9 +651,9 @@ static void piix_set_dmamode (struct ata
 {
 	unsigned int udma	= adev->dma_mode; /* FIXME: MWDMA too */
 	struct pci_dev *dev	= to_pci_dev(ap->host_set->dev);
-	u8 maslave		= ap->hard_port_no ? 0x42 : 0x40;
+	u8 maslave		= ap->port_no ? 0x42 : 0x40;
 	u8 speed		= udma;
-	unsigned int drive_dn	= (ap->hard_port_no ? 2 : 0) + adev->devno;
+	unsigned int drive_dn	= (ap->port_no ? 2 : 0) + adev->devno;
 	int a_speed		= 3 << (drive_dn * 4);
 	int u_flag		= 1 << drive_dn;
 	int v_flag		= 0x01 << drive_dn;

Re: [PATCH/RFC 0/2] libata: legacy mode fixes

[Jeff Garzik]

Albert Lee wrote:
> Dear all,
> 
> Description:
> This is the revised patch by Unicorn and I for legacy mode fix.
> 
> patch 1/2: make both legacy ports share one host_set.
> patch 2/2: remove the unused ap->hard_port_no.

Unfortunately, NAK.

While it would be nice to eventually share one host_set, your patch 
demonstrates the hacks that must be added to the hot path, to deal with 
two separate interrupt handlers trying to share the same host_set.

Additionally, neither this email nor the two patches describes the 
problem you are trying to fix, and how these changes fix that problem.

The correct solution, long term, is
1) move the irq registration (request_irq, free_irq) out of 
ata_device_add().  The current design was convenient initially, but I 
have known since I wrote the code that it was insufficient, long term.

The best solution is to give each LLDD the ability to register an 
interrupt handler in a manner best suited to the hardware.  Consider, 
for example, how poorly the current design will work on PCI MSI-X hardware.

2) For legacy mode hardware, one should pass struct ata_port* to 
request_irq(), rather then struct ata_host_set*.  Thus in the legacy ATA 
interrupt handler, obtain the host_set via ap->host_set.

	Jeff

Re: [PATCH/RFC 0/2] libata: legacy mode fixes

[Albert Lee]

Jeff Garzik wrote:
> Albert Lee wrote:
> 
>> Dear all,
>>
>> Description:
>> This is the revised patch by Unicorn and I for legacy mode fix.
>>
>> patch 1/2: make both legacy ports share one host_set.
>> patch 2/2: remove the unused ap->hard_port_no.
> 
> 
> Unfortunately, NAK.
> 
> While it would be nice to eventually share one host_set, your patch
> demonstrates the hacks that must be added to the hot path, to deal with
> two separate interrupt handlers trying to share the same host_set.
> 
> Additionally, neither this email nor the two patches describes the
> problem you are trying to fix, and how these changes fix that problem.

Sorry for not describing the problem properly. There are two problems
related to "each legacy port has its own host_set":
1. When the LLDD module with legacy ports is unloaded, only
   irq 15 is freed while irq 14 is not. So, when the LLDD module
   is reloaded, it cannot get irq 14 non-shared and fails to load.
2. For simplex device in legacy mode, both primary and secondary
   legacy port will get the DMA engine because the ATA_HOST_SIMPLEX
   flag is available in the host_sets of each legacy port.

The patch tries to fix the probems by
1. Let both legacy ports share the same host_set (fix for problem 2).
2. Add per port irq to ata_ioports for legacy irq 14/15 since
   host_set is shared now.
3. Add irq handler hack to support both per-port legacy irq and
   per adapter shared irq cases.

> 
> The correct solution, long term, is
> 1) move the irq registration (request_irq, free_irq) out of
> ata_device_add().  The current design was convenient initially, but I
> have known since I wrote the code that it was insufficient, long term.
> 
> The best solution is to give each LLDD the ability to register an
> interrupt handler in a manner best suited to the hardware.  Consider,
> for example, how poorly the current design will work on PCI MSI-X hardware.

Hmm, you're right.

> 
> 2) For legacy mode hardware, one should pass struct ata_port* to
> request_irq(), rather then struct ata_host_set*.  Thus in the legacy ATA
> interrupt handler, obtain the host_set via ap->host_set.
> 

Thanks for the advice. Will revise the patch accordingly.
--
albert