[PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Tejun Heo]

host->irq and host->irq2 should be set before ata_host_register() for
IRQ reporting to work.  Move up host->irq assignment in
ata_host_activate() and add it to ata_pci_init_one() native path and
pata_cs5520.

The port info printing in ata_host_register() doesn't fit all the
different controllers.  It should probably be moved out to LLDs with
some helpers in the future.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |    6 +++---
 drivers/ata/libata-sff.c  |    5 +++--
 drivers/ata/pata_cs5520.c |    5 +++++
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bfc59a1..e57a9f8 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6424,14 +6424,14 @@ int ata_host_activate(struct ata_host *host, int irq,
 	if (rc)
 		return rc;
 
+	/* Used to print device info at probe */
+	host->irq = irq;
+
 	rc = ata_host_register(host, sht);
 	/* if failed, just free the IRQ and leave ports alone */
 	if (rc)
 		devm_free_irq(host->dev, irq, host);
 
-	/* Used to print device info at probe */
-	host->irq = irq;
-
 	return rc;
 }
 
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index ce84805..fa1c22c 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1065,10 +1065,11 @@ int ata_pci_init_one(struct pci_dev *pdev,
 	if (rc)
 		goto err_out;
 
-	if (!legacy_mode)
+	if (!legacy_mode) {
 		rc = devm_request_irq(dev, pdev->irq, pi->port_ops->irq_handler,
 				      IRQF_SHARED, DRV_NAME, host);
-	else {
+		host->irq = pdev->irq;
+	} else {
 		irq_handler_t handler[2] = { host->ops->irq_handler,
 					     host->ops->irq_handler };
 		unsigned int irq_flags[2] = { IRQF_SHARED, IRQF_SHARED };
diff --git a/drivers/ata/pata_cs5520.c b/drivers/ata/pata_cs5520.c
index 1aabe15..00cf013 100644
--- a/drivers/ata/pata_cs5520.c
+++ b/drivers/ata/pata_cs5520.c
@@ -284,6 +284,11 @@ static int __devinit cs5520_init_one(struct pci_dev *pdev, const struct pci_devi
 				      ata_interrupt, 0, DRV_NAME, host);
 		if (rc)
 			return rc;
+
+		if (i == 0)
+			host->irq = irq[0];
+		else
+			host->irq2 = irq[1];
 	}
 
 	return ata_host_register(host, &cs5520_sht);

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Francois Romieu]

Tejun Heo <htejun@gmail.com> :
> host->irq and host->irq2 should be set before ata_host_register() for
> IRQ reporting to work.  Move up host->irq assignment in
> ata_host_activate() and add it to ata_pci_init_one() native path and
> pata_cs5520.
> 
> The port info printing in ata_host_register() doesn't fit all the
> different controllers.  It should probably be moved out to LLDs with
> some helpers in the future.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

Is it supposed to change the behavior below ?

[    0.000000] Linux version 2.6.22-rc7 (root@localhost.localdomain) (gcc version 4.1.1 20070105 (Red Hat 4.1.1-51)) #12 Mon Jul 2 22:36:43 CEST 2007
[...]
[   30.322290] SCSI subsystem initialized
[   30.335668] libata version 2.21 loaded.
[   30.338329] ata_piix 0000:00:1f.2: version 2.11
[   30.338341] ata_piix 0000:00:1f.2: MAP [ P0 P2 P1 P3 ]
[   30.343809] ACPI: PCI Interrupt 0000:00:1f.2[B] -> GSI 19 (level, low) -> IRQ 19
[   30.351476] PCI: Setting latency timer of device 0000:00:1f.2 to 64
[   30.351592] scsi0 : ata_piix
[   30.354807] scsi1 : ata_piix
[   30.357975] ata1: SATA max UDMA/133 cmd 0x0001c080 ctl 0x0001c002 bmdma 0x0001b800 irq 0
                                                                                          ^
[   30.366152] ata2: SATA max UDMA/133 cmd 0x0001bc00 ctl 0x0001b882 bmdma 0x0001b808 irq 0
                                                                                          ^

-- 
Ueimor

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Tejun Heo]

Francois Romieu wrote:
> Tejun Heo <htejun@gmail.com> :
>> host->irq and host->irq2 should be set before ata_host_register() for
>> IRQ reporting to work.  Move up host->irq assignment in
>> ata_host_activate() and add it to ata_pci_init_one() native path and
>> pata_cs5520.
>>
>> The port info printing in ata_host_register() doesn't fit all the
>> different controllers.  It should probably be moved out to LLDs with
>> some helpers in the future.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> Is it supposed to change the behavior below ?

Yeah, didn't it?

-- 
tejun

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Francois Romieu]

Tejun Heo <htejun@gmail.com> :
> Francois Romieu wrote:
> > Tejun Heo <htejun@gmail.com> :
[...@
> >> The port info printing in ata_host_register() doesn't fit all the
> >> different controllers.  It should probably be moved out to LLDs with
> >> some helpers in the future.
> >>
> >> Signed-off-by: Tejun Heo <htejun@gmail.com>
> > 
> > Is it supposed to change the behavior below ?
> 
> Yeah, didn't it?

I have rebuilt on top of 872aad45d6174570dd2e1defc3efee50f2cfcc72 and it
still seems bizarre:

[    0.000000] Linux version 2.6.22-rc7 (root@localhost.localdomain) (gcc version 4.1.1 20070105 (Red Hat 4.1.1-51)) #13 Tue Jul 3 20:42:14 CEST 2007
[...]
[   30.140053] input: AT Translated Set 2 keyboard as /class/input/input0
[   30.405873] SCSI subsystem initialized
[   30.419216] libata version 2.21 loaded.
[   30.421854] ata_piix 0000:00:1f.2: version 2.11
[   30.421865] ata_piix 0000:00:1f.2: MAP [ P0 P2 P1 P3 ]
[   30.427343] ACPI: PCI Interrupt 0000:00:1f.2[B] -> GSI 19 (level, low) -> IRQ 19
[   30.435010] PCI: Setting latency timer of device 0000:00:1f.2 to 64
[   30.435123] scsi0 : ata_piix
[   30.438336] scsi1 : ata_piix
[   30.441502] ata1: SATA max UDMA/133 cmd 0x0001c080 ctl 0x0001c002 bmdma 0x0001b800 irq 0
                                                                                          ^
[   30.449680] ata2: SATA max UDMA/133 cmd 0x0001bc00 ctl 0x0001b882 bmdma 0x0001b808 irq 0
                                                                                          ^
[   30.658396] ata1.00: ata_hpa_resize 1: sectors = 312581808, hpa_sectors = 312581808
[   30.666137] ata1.00: ATA-7: MAXTOR STM3160211AS, 3.AAE, max UDMA/133
[   30.672537] ata1.00: 312581808 sectors, multi 16: LBA48 NCQ (depth 0/32)
[   30.733289] ata1.00: ata_hpa_resize 1: sectors = 312581808, hpa_sectors = 312581808
[   30.741026] ata1.00: configured for UDMA/133

As Alan said, it is not a serious problem though.

-- 
Ueimor

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Tejun Heo]

Francois Romieu wrote:
> I have rebuilt on top of 872aad45d6174570dd2e1defc3efee50f2cfcc72 and it
> still seems bizarre:
> 
> [    0.000000] Linux version 2.6.22-rc7 (root@localhost.localdomain) (gcc version 4.1.1 20070105 (Red Hat 4.1.1-51)) #13 Tue Jul 3 20:42:14 CEST 2007
> [...]
> [   30.140053] input: AT Translated Set 2 keyboard as /class/input/input0
> [   30.405873] SCSI subsystem initialized
> [   30.419216] libata version 2.21 loaded.
> [   30.421854] ata_piix 0000:00:1f.2: version 2.11
> [   30.421865] ata_piix 0000:00:1f.2: MAP [ P0 P2 P1 P3 ]
> [   30.427343] ACPI: PCI Interrupt 0000:00:1f.2[B] -> GSI 19 (level, low) -> IRQ 19
> [   30.435010] PCI: Setting latency timer of device 0000:00:1f.2 to 64
> [   30.435123] scsi0 : ata_piix
> [   30.438336] scsi1 : ata_piix
> [   30.441502] ata1: SATA max UDMA/133 cmd 0x0001c080 ctl 0x0001c002 bmdma 0x0001b800 irq 0
>                                                                                           ^
> [   30.449680] ata2: SATA max UDMA/133 cmd 0x0001bc00 ctl 0x0001b882 bmdma 0x0001b808 irq 0
>                                                                                           ^
> [   30.658396] ata1.00: ata_hpa_resize 1: sectors = 312581808, hpa_sectors = 312581808
> [   30.666137] ata1.00: ATA-7: MAXTOR STM3160211AS, 3.AAE, max UDMA/133
> [   30.672537] ata1.00: 312581808 sectors, multi 16: LBA48 NCQ (depth 0/32)
> [   30.733289] ata1.00: ata_hpa_resize 1: sectors = 312581808, hpa_sectors = 312581808
> [   30.741026] ata1.00: configured for UDMA/133
> 
> As Alan said, it is not a serious problem though.

Seriously stupid tho. :-) I've tested ata_piix.  What did I miss?  I'll
look into it.

-- 
tejun

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Tejun Heo]

Tejun Heo wrote:
> Francois Romieu wrote:
>> I have rebuilt on top of 872aad45d6174570dd2e1defc3efee50f2cfcc72 and it
>> still seems bizarre:

Okay, 872aad45 doesn't have the patch yet.  Either apply the patch
manually or you need to wait a bit more.

-- 
tejun

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Francois Romieu]

Tejun Heo <htejun@gmail.com> :
> Tejun Heo wrote:
> > Francois Romieu wrote:
> >> I have rebuilt on top of 872aad45d6174570dd2e1defc3efee50f2cfcc72 and it
> >> still seems bizarre:
> 
> Okay, 872aad45 doesn't have the patch yet.  Either apply the patch
> manually or you need to wait a bit more.

I am confused. 4031826b3ca40982880f6b9f2282c7d7fad60d77 appears in the log:
Author: Tejun Heo <htejun@gmail.com>                                            
Date:   Tue Jul 3 01:38:47 2007 +0900                                           
                                                                                
    libata: fix assigned IRQ reporting                                          
                                                                                
    host->irq and host->irq2 should be set before ata_host_register() for       
    IRQ reporting to work.  Move up host->irq assignment in                     
    ata_host_activate() and add it to ata_pci_init_one() native path and        
    pata_cs5520.                                                                
                                                                                
    The port info printing in ata_host_register() doesn't fit all the           
    different controllers.  It should probably be moved out to LLDs with        
    some helpers in the future.                                                 
                                                                                
    Signed-off-by: Tejun Heo <htejun@gmail.com>                                 
    Signed-off-by: Jeff Garzik <jeff@garzik.org>                                

-> it is exactly the patch that you sent on 03/07/2007.

-- 
Ueimor

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Tejun Heo]

Francois Romieu wrote:
> Tejun Heo <htejun@gmail.com> :
>> Tejun Heo wrote:
>>> Francois Romieu wrote:
>>>> I have rebuilt on top of 872aad45d6174570dd2e1defc3efee50f2cfcc72 and it
>>>> still seems bizarre:
>> Okay, 872aad45 doesn't have the patch yet.  Either apply the patch
>> manually or you need to wait a bit more.
> 
> I am confused. 4031826b3ca40982880f6b9f2282c7d7fad60d77 appears in the log:
> Author: Tejun Heo <htejun@gmail.com>                                            
> Date:   Tue Jul 3 01:38:47 2007 +0900                                           
>                                                                                 
>     libata: fix assigned IRQ reporting                                          

Apparently, I'm more confused than you are.  My "master" was at 872aad45
but my index was at rc7.  Can you please force checkout the head and
re-test?  I've tested both native and legacy modes and it's fixed here.

Thanks.

-- 
tejun

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Francois Romieu]

[-- Attachment #1: Type: text/plain, Size: 383 bytes --]

Tejun Heo <htejun@gmail.com> :
[...]
> Apparently, I'm more confused than you are.  My "master" was at 872aad45
> but my index was at rc7.  Can you please force checkout the head and
> re-test?  I've tested both native and legacy modes and it's fixed here.

I have done a forced checkout + mrproper before rebuild and there is no
difference.

Complete dmesg is attached.

-- 
Ueimor

[-- Attachment #2: dmesg-20070705.gz --]
[-- Type: application/x-gzip, Size: 9071 bytes --]

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Tejun Heo]

[-- Attachment #1: Type: text/plain, Size: 510 bytes --]

Francois Romieu wrote:
> Tejun Heo <htejun@gmail.com> :
> [...]
>> Apparently, I'm more confused than you are.  My "master" was at 872aad45
>> but my index was at rc7.  Can you please force checkout the head and
>> re-test?  I've tested both native and legacy modes and it's fixed here.
> 
> I have done a forced checkout + mrproper before rebuild and there is no
> difference.
> 
> Complete dmesg is attached.

Hmmmm... Weird.  Can you please apply the attached patch and post boot
dmesg?  Thanks.

-- 
tejun

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 786 bytes --]

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index fa1c22c..1a5bd75 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1069,6 +1069,7 @@ int ata_pci_init_one(struct pci_dev *pdev,
 		rc = devm_request_irq(dev, pdev->irq, pi->port_ops->irq_handler,
 				      IRQF_SHARED, DRV_NAME, host);
 		host->irq = pdev->irq;
+		printk("XXX native mode irq=%lu\n", host->irq);
 	} else {
 		irq_handler_t handler[2] = { host->ops->irq_handler,
 					     host->ops->irq_handler };
@@ -1076,6 +1077,7 @@ int ata_pci_init_one(struct pci_dev *pdev,
 		void *dev_id[2] = { host, host };
 
 		rc = ata_request_legacy_irqs(host, handler, irq_flags, dev_id);
+		printk("XXX legacy mode irq=%lu irq2=%lu\n", host->irq, host->irq2);
 	}
 	if (rc)
 		goto err_out;

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Francois Romieu]

Tejun Heo <htejun@gmail.com> :
[...]
> Hmmmm... Weird.  Can you please apply the attached patch and post boot
> dmesg?  Thanks.

It works perfectly in 2.6.22 but the .config is not the same.

I'll report if it makes a difference.

-- 
Ueimor

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Francois Romieu]

Francois Romieu <romieu@fr.zoreil.com> :
[...]
> It works perfectly in 2.6.22 but the .config is not the same.
> 
> I'll report if it makes a difference.

False alarm: I did not update my initrd correctly after 2.6.22-rc6.
The patch worked correctly.

Sorry for the wasted time.

-- 
Ueimor

Re: [PATCH 2.6.22-rc7] libata: fix assigned IRQ reporting

[Jeff Garzik]

Tejun Heo wrote:
> host->irq and host->irq2 should be set before ata_host_register() for
> IRQ reporting to work.  Move up host->irq assignment in
> ata_host_activate() and add it to ata_pci_init_one() native path and
> pata_cs5520.
> 
> The port info printing in ata_host_register() doesn't fit all the
> different controllers.  It should probably be moved out to LLDs with
> some helpers in the future.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/libata-core.c |    6 +++---
>  drivers/ata/libata-sff.c  |    5 +++--
>  drivers/ata/pata_cs5520.c |    5 +++++
>  3 files changed, 11 insertions(+), 5 deletions(-)

applied