Patchwork [11/12] libata: use IRQ expecting

login
register
mail settings
Submitter Tejun Heo
Date June 13, 2010, 3:31 p.m.
Message ID <1276443098-20653-12-git-send-email-tj@kernel.org>
Download mbox | patch
Permalink /patch/55450/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Tejun Heo - June 13, 2010, 3:31 p.m.
Thanks to its age, ATA is very susceptible to IRQ delivery problems in
both directions - lost and spurious interrupts.  In traditional PATA,
the IRQ line is ultimately out of the controller and driver's control.
Even relatively new SATA isn't free from these issues.  Many
controllers still emulate the traditional IDE interface which doesn't
have reliable way to indicate interrupt pending state and there also
is an issue regarding the interpretation of nIEN on both sides of the
cable.

Most of these problems can be worked around by using the new IRQ
expecting mechanism without adding noticeable overhead.  In ATA,
almost all operations are initiated by the host and the controller
signals progress or completion using IRQ.  IRQ expecting can easily be
added in libata core and applied to all libata drivers.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/libata-core.c |   15 +++++++++++++--
 drivers/ata/libata-eh.c   |    4 +++-
 drivers/ata/libata-sff.c  |   37 +++++++++++++++++++------------------
 include/linux/libata.h    |    2 ++
 4 files changed, 37 insertions(+), 21 deletions(-)
Tejun Heo - June 21, 2010, 1:52 p.m.
On 06/13/2010 05:31 PM, Tejun Heo wrote:
> Thanks to its age, ATA is very susceptible to IRQ delivery problems in
> both directions - lost and spurious interrupts.  In traditional PATA,
> the IRQ line is ultimately out of the controller and driver's control.
> Even relatively new SATA isn't free from these issues.  Many
> controllers still emulate the traditional IDE interface which doesn't
> have reliable way to indicate interrupt pending state and there also
> is an issue regarding the interpretation of nIEN on both sides of the
> cable.
> 
> Most of these problems can be worked around by using the new IRQ
> expecting mechanism without adding noticeable overhead.  In ATA,
> almost all operations are initiated by the host and the controller
> signals progress or completion using IRQ.  IRQ expecting can easily be
> added in libata core and applied to all libata drivers.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>

Jeff, can you please ack this change if this looks okay to you?  Also,
would it okay to route this through irq tree?

Thanks.
Jeff Garzik - June 25, 2010, 12:22 a.m.
On 06/13/2010 11:31 AM, Tejun Heo wrote:
> Thanks to its age, ATA is very susceptible to IRQ delivery problems in
> both directions - lost and spurious interrupts.  In traditional PATA,
> the IRQ line is ultimately out of the controller and driver's control.
> Even relatively new SATA isn't free from these issues.  Many
> controllers still emulate the traditional IDE interface which doesn't
> have reliable way to indicate interrupt pending state and there also
> is an issue regarding the interpretation of nIEN on both sides of the
> cable.
>
> Most of these problems can be worked around by using the new IRQ
> expecting mechanism without adding noticeable overhead.  In ATA,
> almost all operations are initiated by the host and the controller
> signals progress or completion using IRQ.  IRQ expecting can easily be
> added in libata core and applied to all libata drivers.
>
> Signed-off-by: Tejun Heo<tj@kernel.org>
> ---
>   drivers/ata/libata-core.c |   15 +++++++++++++--
>   drivers/ata/libata-eh.c   |    4 +++-
>   drivers/ata/libata-sff.c  |   37 +++++++++++++++++++------------------
>   include/linux/libata.h    |    2 ++
>   4 files changed, 37 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index ddf8e48..9a0aaa0 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4972,6 +4972,8 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
>   {
>   	struct ata_port *ap = qc->ap;
>
> +	unexpect_irq(ap->irq_expect, false);
> +
>   	/* XXX: New EH and old EH use different mechanisms to
>   	 * synchronize EH with regular execution path.
>   	 *

Unconditional use of unexpect_irq() here seems incorrect for some cases, 
such as sata_mv's use, where ata_qc_complete() is called multiple times 
rather than a singleton ata_qc_complete_multiple() call.

	Jeff






--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - June 25, 2010, 7:44 a.m.
Hello, Jeff.

On 06/25/2010 02:22 AM, Jeff Garzik wrote:
>> @@ -4972,6 +4972,8 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
>>   {
>>       struct ata_port *ap = qc->ap;
>>
>> +    unexpect_irq(ap->irq_expect, false);
>> +
>>       /* XXX: New EH and old EH use different mechanisms to
>>        * synchronize EH with regular execution path.
>>        *
> 
> Unconditional use of unexpect_irq() here seems incorrect for some cases,
> such as sata_mv's use, where ata_qc_complete() is called multiple times
> rather than a singleton ata_qc_complete_multiple() call.

Indeed, sata_mv is calling ata_qc_complete() directly multiple times.
I still think calling unexpect_irq() from ata_qc_complete() is correct
as ata_qc_complete() is always a good indicator of completion events.
What's missing is a way for sata_mv to indicate that it has more
events to expect for, which under the current implementation only
sata_mv interrupt handler can determine.  I'll see if I can convert it
to use ata_qc_complete_multiple() instead.

Thanks.
Jeff Garzik - June 25, 2010, 9:48 a.m.
On 06/25/2010 03:44 AM, Tejun Heo wrote:
> Hello, Jeff.
>
> On 06/25/2010 02:22 AM, Jeff Garzik wrote:
>>> @@ -4972,6 +4972,8 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
>>>    {
>>>        struct ata_port *ap = qc->ap;
>>>
>>> +    unexpect_irq(ap->irq_expect, false);
>>> +
>>>        /* XXX: New EH and old EH use different mechanisms to
>>>         * synchronize EH with regular execution path.
>>>         *
>>
>> Unconditional use of unexpect_irq() here seems incorrect for some cases,
>> such as sata_mv's use, where ata_qc_complete() is called multiple times
>> rather than a singleton ata_qc_complete_multiple() call.
>
> Indeed, sata_mv is calling ata_qc_complete() directly multiple times.
> I still think calling unexpect_irq() from ata_qc_complete() is correct
> as ata_qc_complete() is always a good indicator of completion events.

My basic point is that you are implicitly changing the entire 
ata_qc_complete() API, and associated underlying assumptions.

The existing assumption, since libata day #0, is that ata_qc_complete() 
works entirely within the scope of a single qc -- thus enabling multiple 
calls for a single controller interrupt.  Your change greatly widens the 
scope to an entire port.

This isn't just an issue with sata_mv, that was just the easy example I 
remember off the top of my head.  sata_fsl and sata_nv also make the 
same assumption.  And it's a reasonable assumption, IMO.

I think an unexpect_irq() call is more appropriate outside 
ata_qc_complete().

	Jeff



--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - June 25, 2010, 9:51 a.m.
Hello,

On 06/25/2010 11:48 AM, Jeff Garzik wrote:
> My basic point is that you are implicitly changing the entire
> ata_qc_complete() API, and associated underlying assumptions.
> 
> The existing assumption, since libata day #0, is that ata_qc_complete()
> works entirely within the scope of a single qc -- thus enabling multiple
> calls for a single controller interrupt.  Your change greatly widens the
> scope to an entire port.

Yeah, I'm changing that and it actually reduces code.

> This isn't just an issue with sata_mv, that was just the easy example I
> remember off the top of my head.  sata_fsl and sata_nv also make the
> same assumption.  And it's a reasonable assumption, IMO.

Yeah, already updating all of them.

> I think an unexpect_irq() call is more appropriate outside
> ata_qc_complete().

The choices we have here are....

1. Update completion API so that libata core layer has enough
   information to decide expect/unexpect events.

2. Add expect/unexpect calls to individual drivers.

I think #1 is much better now and in the long run.  The code actually
looks better too.

Thanks.
Jeff Garzik - June 26, 2010, 3:45 a.m.
On 06/25/2010 03:44 AM, Tejun Heo wrote:
> I still think calling unexpect_irq() from ata_qc_complete() is correct
> as ata_qc_complete() is always a good indicator of completion events.

No, it's not.  ata_qc_complete() is an indicator that _one_ completion 
event occurred, not _all_ completion events for that port.

Converting drivers to use ata_qc_complete_multiple() completely misses 
the point:  ata_qc_complete_multiple() is doing exactly the same thing 
as those drivers:  calling ata_qc_complete() in a loop.

ata_qc_complete() is -- as its name implies -- completing a single qc. 
Your patch introduces an unconditional controller-wide unexpect_irq() 
call.  It's a layering violation.

You can trivially trace through ata_qc_complete_multiple() after patch 
#11 is applied, and see the result...  for $N completion bits passed to 
ata_qc_complete_multiple(), you call
	unexpect_irq()
	expect_irq()
in rapid succession $N times, once for each ata_qc_complete() call in 
the loop of ata_qc_complete_multiple().  For something that is not 
needed for modern SATA controllers.

The preferred solution would be something that only touches legacy 
controllers, namely:

*) create ata_port_complete(), with the implementation

	ata_qc_complete()
	unexpect_irq()

*) then call ata_port_complete() in the legacy code that needs 
unexpect_irq()

We don't want to  burden modern SATA drivers with the overhead of 
dealing with silly PATA/SATA1 legacy irq nastiness, particularly the 
ugliness of calling

	unexpect_irq() + expect_irq()

for a number of NCQ commands, in a tight loop!

	Jeff





--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Garzik - June 26, 2010, 3:52 a.m.
On 06/25/2010 11:45 PM, Jeff Garzik wrote:
> ata_qc_complete_multiple(), you call
> unexpect_irq()
> expect_irq()


Correction,

	unexpect_irq() x N
	expect_irq()

for N NCQ commands.

Controller-wide irq twiddling in the function that completes a single 
ATA command is painful.

	Jeff


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - June 26, 2010, 8:31 a.m.
Hello, Jeff.

On 06/26/2010 05:45 AM, Jeff Garzik wrote:
> No, it's not.  ata_qc_complete() is an indicator that _one_ completion
> event occurred, not _all_ completion events for that port.

Well, it can indicte the start of cluster of completions, which is the
necessary information anyway.  From the second call on, it's a simple
flag test and return.  I doubt it will affect anything even w/ high
performance SSDs but please read on.

> Converting drivers to use ata_qc_complete_multiple() completely misses
> the point:  ata_qc_complete_multiple() is doing exactly the same thing
> as those drivers:  calling ata_qc_complete() in a loop.

The point of ata_qc_complete_multiple() is giving libata core layer
better visibility into how commands are being executed, which in turn
allows (continued below)

> ata_qc_complete() is -- as its name implies -- completing a single qc.
> Your patch introduces an unconditional controller-wide unexpect_irq()
> call.  It's a layering violation.
> 
> You can trivially trace through ata_qc_complete_multiple() after patch
> #11 is applied, and see the result...  for $N completion bits passed to
> ata_qc_complete_multiple(), you call
>     unexpect_irq()
>     expect_irq()
> in rapid succession $N times, once for each ata_qc_complete() call in
> the loop of ata_qc_complete_multiple().  For something that is not
> needed for modern SATA controllers.

ata_qc_complete_multiple() call [un]expect_irq() only once by
introducing an internal completion function w/o irq expect handling,
say ata_qc_complete_raw() and making both ata_qc_complete() and
ata_qc_complete_multiple() simple wrapper around it w/ irq expect
handling.

> The preferred solution would be something that only touches legacy
> controllers, namely:
> 
> *) create ata_port_complete(), with the implementation
> 
>     ata_qc_complete()
>     unexpect_irq()
> 
> *) then call ata_port_complete() in the legacy code that needs
> unexpect_irq()
> 
> We don't want to  burden modern SATA drivers with the overhead of
> dealing with silly PATA/SATA1 legacy irq nastiness, particularly the
> ugliness of calling

I think we're much better off applying it to all the drivers.  IRQ
expecting is very cheap and scalable and there definitely are plenty
of IRQ delivery problems with modern controllers although their
patterns tend to be different from legacy ones.  Plus, it will also be
useful for power state predictions.

>     unexpect_irq() + expect_irq()
> 
> for a number of NCQ commands, in a tight loop!

As I wrote above, I don't think N*unexpect_irq() really matters but
with ata_qc_complete_multiple() conversion, there will only be single
pair of expect/unexpect for each cluster of completions anyway.

Thanks.
Jeff Garzik - June 26, 2010, 9:16 a.m.
On 06/26/2010 04:31 AM, Tejun Heo wrote:
> Well, it can indicte the start of cluster of completions, which is the
> necessary information anyway.  From the second call on, it's a simple
> flag test and return.  I doubt it will affect anything even w/ high
> performance SSDs but please read on.

Yes, and your patch calls unexpect_irq() at the _start_ of a cluster of 
completions.  That is nonsensical, because it reflects the /opposite/ of 
the present ATA bus state, when multiple commands are in flight.


> ata_qc_complete_multiple() call [un]expect_irq() only once by
> introducing an internal completion function w/o irq expect handling,
> say ata_qc_complete_raw() and making both ata_qc_complete() and
> ata_qc_complete_multiple() simple wrapper around it w/ irq expect
> handling.

Yes, this fixes problem, but it is better to create a wrapper path for 
the legacy PATA/SATA1 that uses irq-expecting, and a fast path for 
modern controllers that do not use it.


> On 06/26/2010 05:45 AM, Jeff Garzik wrote:
>> We don't want to  burden modern SATA drivers with the overhead of
>> dealing with silly PATA/SATA1 legacy irq nastiness, particularly the
>> ugliness of calling
>
> I think we're much better off applying it to all the drivers.  IRQ
> expecting is very cheap and scalable and there definitely are plenty
> of IRQ delivery problems with modern controllers although their
> patterns tend to be different from legacy ones.  Plus, it will also be
> useful for power state predictions.

Modern SATA/SAS controllers, and their drivers, already have well 
defined methods of acknowledging interrupts, even unexpected ones, in 
ways that do not need this core manipulation.  This is over-engineering, 
punishing all modern chipsets moving forward regardless of their design, 
by unconditionally requiring this behavior of all libata drivers.

Just like the rest of libata's layered driver architecture, it should be 
straightforward to apply this only to SFF/BMDMA chipsets, then tackle 
odd cases as needs arise.

Modern controllers acknowledge interrupts sanely, and always "expect" an 
interrupt when you include interrupt events like hotplug, even if the 
ATA bus itself is idle.  There is no need to burden the millions of ahci 
users with irq-expecting, for example.

With regards to power state predictions, it is only useful if you are 
accurately reflecting the ATA bus state (idle or not) at all times.  As 
mentioned above, this patch clearly creates a condition where 
unexpect_irq() is called when commands remain in flight, and libata is 
expecting further command completions.

IOW, patch #11 says "we are not expecting irq" when we are.

At least a halfway sane approach would be to track bus-idle status, and 
trigger useful code when that status changes (idle->active or 
active->idle).  Perhaps LED, power state, and irq-expecting could all 
use such a triggering mechanism.

	Jeff



--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - June 26, 2010, 9:44 a.m.
Hello, Jeff.

On 06/26/2010 11:16 AM, Jeff Garzik wrote:
> On 06/26/2010 04:31 AM, Tejun Heo wrote:
>> Well, it can indicte the start of cluster of completions, which is the
>> necessary information anyway.  From the second call on, it's a simple
>> flag test and return.  I doubt it will affect anything even w/ high
>> performance SSDs but please read on.
> 
> Yes, and your patch calls unexpect_irq() at the _start_ of a cluster of
> completions.  That is nonsensical, because it reflects the /opposite/ of
> the present ATA bus state, when multiple commands are in flight.

That's actually what we wanna know.  I'll talk about it below.

>> ata_qc_complete_multiple() call [un]expect_irq() only once by
>> introducing an internal completion function w/o irq expect handling,
>> say ata_qc_complete_raw() and making both ata_qc_complete() and
>> ata_qc_complete_multiple() simple wrapper around it w/ irq expect
>> handling.
> 
> Yes, this fixes problem, but it is better to create a wrapper path for
> the legacy PATA/SATA1 that uses irq-expecting, and a fast path for
> modern controllers that do not use it.
> 
>> On 06/26/2010 05:45 AM, Jeff Garzik wrote:
>>> We don't want to  burden modern SATA drivers with the overhead of
>>> dealing with silly PATA/SATA1 legacy irq nastiness, particularly the
>>> ugliness of calling
>>
>> I think we're much better off applying it to all the drivers.  IRQ
>> expecting is very cheap and scalable and there definitely are plenty
>> of IRQ delivery problems with modern controllers although their
>> patterns tend to be different from legacy ones.  Plus, it will also be
>> useful for power state predictions.
> 
> Modern SATA/SAS controllers, and their drivers, already have well
> defined methods of acknowledging interrupts, even unexpected ones, in
> ways that do not need this core manipulation.  This is over-engineering,
> punishing all modern chipsets moving forward regardless of their design,
> by unconditionally requiring this behavior of all libata drivers.

Unacked irqs are primarily handled by spurious IRQ handling.  IRQ
expecting is more about lost interrupts and we have enough lost
interrupt cases even on new controllers w/ native interface, both
transient and non-transient.

One of the goals of this whole IRQ exception handling was to make it
dumb easy for drivers to use which also included makes things cheap
enough so that they can be called from hot paths.  Both expect and
unexpect_irq() are very cheap once the IRQ delivery is verified.  If
the processor is taking an interrupt in the first place, this amount
of logic shouldn't matter at all.  There really isn't punishment to
avoid and IMHO not doing it for native controllers is an over
optimization.  It gains almost nothing while losing meaningful
protection.

> Just like the rest of libata's layered driver architecture, it should be
> straightforward to apply this only to SFF/BMDMA chipsets, then tackle
> odd cases as needs arise.
>
> Modern controllers acknowledge interrupts sanely, and always "expect" an
> interrupt when you include interrupt events like hotplug, even if the
> ATA bus itself is idle.  There is no need to burden the millions of ahci
> users with irq-expecting, for example.

I'm not saying applying it to only SFF/BMDMA is difficult, just that
it's better to apply it to all drivers in this case.  IRQ expecting is
to protect against misdelivered / lost IRQs and we do have them for
ahci, sil24 or whatever too.  It would of course be silly to pay
significant performance overhead for such protection but as I stated
above, it's _really_ cheap.  If the driver is taking an interrupt and
accessing harddware and even if compared only against the general
complexity of generic IRQ and libata code, the cost of IRQ [un]expect
is negligible and designed precisely that way to allow use cases like
this.

> With regards to power state predictions, it is only useful if you are
> accurately reflecting the ATA bus state (idle or not) at all times.  As
> mentioned above, this patch clearly creates a condition where
> unexpect_irq() is called when commands remain in flight, and libata is
> expecting further command completions.
>
> IOW, patch #11 says "we are not expecting irq" when we are.
> 
> At least a halfway sane approach would be to track bus-idle status, and
> trigger useful code when that status changes (idle->active or
> active->idle).  Perhaps LED, power state, and irq-expecting could all
> use such a triggering mechanism.

Continuing from the response to the first paragraph.  The IRQ
expecting code isn't interested in the bus state, it's interested only
in the IRQ events and that's what it's expecting.  The same applies to
power state prediction too, so please consider the following NCQ
command execution sequence.

1. issue tags 0, 1, 2, 3
2. IRQ triggers, tags 0, 2 complete
3. IRQ triggers, tags 1, 3 completes

For IRQ expecting, both 1-2 and 2-3 are segments to expect for and for
power state transition too, as it's IRQ itself which forces the cpu to
come out of sleep state.  The reason why I said unexpect in
ata_qc_complete() is okay is that it can still delimit each segment as
long as we have proper irq_expect() call at the beginning of each
segment (all other unexpect calls are ignored).  But, that's kind of
moot point as we can easily do single pair.

Thanks.

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index ddf8e48..9a0aaa0 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4972,6 +4972,8 @@  void ata_qc_complete(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
 
+	unexpect_irq(ap->irq_expect, false);
+
 	/* XXX: New EH and old EH use different mechanisms to
 	 * synchronize EH with regular execution path.
 	 *
@@ -5087,6 +5089,9 @@  int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
 		done_mask &= ~(1 << tag);
 	}
 
+	if (ap->qc_active)
+		expect_irq(ap->irq_expect);
+
 	return nr_done;
 }
 
@@ -5153,6 +5158,7 @@  void ata_qc_issue(struct ata_queued_cmd *qc)
 	qc->err_mask |= ap->ops->qc_issue(qc);
 	if (unlikely(qc->err_mask))
 		goto err;
+	expect_irq(ap->irq_expect);
 	return;
 
 sg_err:
@@ -6185,8 +6191,13 @@  int ata_host_activate(struct ata_host *host, int irq,
 	if (rc)
 		return rc;
 
-	for (i = 0; i < host->n_ports; i++)
-		ata_port_desc(host->ports[i], "irq %d", irq);
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+
+		if (!ata_port_is_dummy(ap))
+			ap->irq_expect = init_irq_expect(irq, host);
+		ata_port_desc(ap, "irq %d%s", irq, ap->irq_expect ? "+" : "");
+	}
 
 	rc = ata_host_register(host, sht);
 	/* if failed, just free the IRQ and leave ports alone */
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index f77a673..f1ae3ec 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -619,8 +619,10 @@  void ata_scsi_error(struct Scsi_Host *host)
 		 * handler doesn't diddle with those qcs.  This must
 		 * be done atomically w.r.t. setting QCFLAG_FAILED.
 		 */
-		if (nr_timedout)
+		if (nr_timedout) {
+			unexpect_irq(ap->irq_expect, true);
 			__ata_port_freeze(ap);
+		}
 
 		spin_unlock_irqrestore(ap->lock, flags);
 
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index efa4a18..cc96f36 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -2388,7 +2388,8 @@  int ata_pci_sff_activate_host(struct ata_host *host,
 	struct device *dev = host->dev;
 	struct pci_dev *pdev = to_pci_dev(dev);
 	const char *drv_name = dev_driver_string(host->dev);
-	int legacy_mode = 0, rc;
+	struct ata_port *ap[2] = { host->ports[0], host->ports[1] };
+	int legacy_mode = 0, i, rc;
 
 	rc = ata_host_start(host);
 	if (rc)
@@ -2422,29 +2423,29 @@  int ata_pci_sff_activate_host(struct ata_host *host,
 		if (rc)
 			goto out;
 
-		ata_port_desc(host->ports[0], "irq %d", pdev->irq);
-		ata_port_desc(host->ports[1], "irq %d", pdev->irq);
+		for (i = 0; i < 2; i++) {
+			if (!ata_port_is_dummy(ap[i]))
+				ap[i]->irq_expect =
+					init_irq_expect(pdev->irq, host);
+			ata_port_desc(ap[i], "irq %d%s",
+				      pdev->irq, ap[i]->irq_expect ? "+" : "");
+		}
 	} else if (legacy_mode) {
-		if (!ata_port_is_dummy(host->ports[0])) {
-			rc = devm_request_irq(dev, ATA_PRIMARY_IRQ(pdev),
-					      irq_handler, IRQF_SHARED,
-					      drv_name, host);
-			if (rc)
-				goto out;
+		unsigned int irqs[2] = { ATA_PRIMARY_IRQ(pdev),
+					 ATA_SECONDARY_IRQ(pdev) };
 
-			ata_port_desc(host->ports[0], "irq %d",
-				      ATA_PRIMARY_IRQ(pdev));
-		}
+		for (i = 0; i < 2; i++) {
+			if (ata_port_is_dummy(ap[i]))
+				continue;
 
-		if (!ata_port_is_dummy(host->ports[1])) {
-			rc = devm_request_irq(dev, ATA_SECONDARY_IRQ(pdev),
-					      irq_handler, IRQF_SHARED,
-					      drv_name, host);
+			rc = devm_request_irq(dev, irqs[i], irq_handler,
+					      IRQF_SHARED, drv_name, host);
 			if (rc)
 				goto out;
 
-			ata_port_desc(host->ports[1], "irq %d",
-				      ATA_SECONDARY_IRQ(pdev));
+			ap[i]->irq_expect = init_irq_expect(irqs[i], host);
+			ata_port_desc(ap[i], "irq %d%s",
+				      irqs[i], ap[i]->irq_expect ? "+" : "");
 		}
 	}
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index b85f3ff..3f5f159 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -751,6 +751,8 @@  struct ata_port {
 	struct ata_host		*host;
 	struct device 		*dev;
 
+	struct irq_expect	*irq_expect;	/* for irq expecting */
+
 	struct delayed_work	hotplug_task;
 	struct work_struct	scsi_rescan_task;