[#upstream-fixes,1/2] libata: cleanup ata_sff_interrupt()

host->ports[i] is never NULL if i < host->n_ports and non-NULL return
from ata_qc_from_tag() guarantees that the returned qc is active.
Drop unnecessary tests.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/libata-sff.c |   17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

--
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

Hello.

Tejun Heo wrote:

> host->ports[i] is never NULL if i < host->n_ports and non-NULL return
> from ata_qc_from_tag() guarantees that the returned qc is active.
> Drop unnecessary tests.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
>  drivers/ata/libata-sff.c |   17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
>
> Index: ata/drivers/ata/libata-sff.c
> ===================================================================
> --- ata.orig/drivers/ata/libata-sff.c
> +++ ata/drivers/ata/libata-sff.c
> @@ -1767,18 +1767,15 @@ irqreturn_t ata_sff_interrupt(int irq, v
>  	spin_lock_irqsave(&host->lock, flags);
>  
>  	for (i = 0; i < host->n_ports; i++) {
> -		struct ata_port *ap;
> +		struct ata_port *ap = host->ports[i];
> +		struct ata_queued_cmd *qc;
>  
> -		ap = host->ports[i];
> -		if (ap &&
> -		    !(ap->flags & ATA_FLAG_DISABLED)) {
> -			struct ata_queued_cmd *qc;
> +		if (unlikely(ap->flags & ATA_FLAG_DISABLED))
> +			continue;
>  
> -			qc = ata_qc_from_tag(ap, ap->link.active_tag);
> -			if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)) &&
> -			    (qc->flags & ATA_QCFLAG_ACTIVE))
> -				handled |= ata_sff_host_intr(ap, qc);
> -		}
> +		qc = ata_qc_from_tag(ap, ap->link.active_tag);
> +		if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)))
>   

   () not needed around !x.

WBR, Sergei


--
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

On 01/14/2010 02:50 AM, Tejun Heo wrote:
> host->ports[i] is never NULL if i<  host->n_ports and non-NULL return
> from ata_qc_from_tag() guarantees that the returned qc is active.
> Drop unnecessary tests.
>
> Signed-off-by: Tejun Heo<tj@kernel.org>
> ---
>   drivers/ata/libata-sff.c |   17 +++++++----------
>   1 file changed, 7 insertions(+), 10 deletions(-)

This is more #upstream material, don't you think?  We are pretty deep 
into -rc at this point.

(headed out for the weekend, fyi)

	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

On 01/15/2010 02:38 AM, Jeff Garzik wrote:
> On 01/14/2010 02:50 AM, Tejun Heo wrote:
>> host->ports[i] is never NULL if i<  host->n_ports and non-NULL return
>> from ata_qc_from_tag() guarantees that the returned qc is active.
>> Drop unnecessary tests.
>>
>> Signed-off-by: Tejun Heo<tj@kernel.org>
>> ---
>>   drivers/ata/libata-sff.c |   17 +++++++----------
>>   1 file changed, 7 insertions(+), 10 deletions(-)
> 
> This is more #upstream material, don't you think?  We are pretty deep
> into -rc at this point.

Yeap, agreed.

Thanks.

On 01/14/2010 07:20 PM, Sergei Shtylyov wrote:
>> +        qc = ata_qc_from_tag(ap, ap->link.active_tag);
>> +        if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)))
>>   
> 
>   () not needed around !x.

Eh... I don't know.  I personally prefer using minimum number of
parantheses but quite a few people dislike not using them when && & ||
| are mixed.  In the end, I don't think it really matters.  It's like
debating about how far the second part of a broken line should be
indented.  Many people strong opinions but in the end it doesn't
really matter.  Anyhow, if it bothers you enough to write an email
about it, I'll be happy to oblige.  :-)

Thanks.