Re: [PATCH/RFC 5/9] libata: use freeze()/thaw() for polling

[Tejun Heo <htejun@gmail.com>]

Hello,

Albert Lee wrote:
> Patch 5/9:
> 
>   This patch changes polling codes to use freeze()/thaw() for irq off/on.
> ata_qc_set_polling() is also removed since now unused.
> 
> The reason for freeze()/thaw(): some ATAPI devices raises INTRQ even if nIEN = 1.
> Using the host adapter irq mask mechanism, if available, is more reliable than using the device nIEN bit.
> 
> Considerations:
>  1. the semantic of freeze()/thaw() maybe more than irq off/on?

It usually is irq off/on.

>  2. HSM, the new user of freeze()/thaw(), will call freeze()/thaw() more frequently than EH.
>     Can current implemention of freeze()/thaw() handle it?

Yeah, I don't see any reason why it can't be used that way but please
audit each freeze/thaw implementation.

> @@ -5442,7 +5442,7 @@ unsigned int ata_qc_issue_prot(struct at
>  
>  	case ATA_PROT_PIO:
>  		if (qc->tf.flags & ATA_TFLAG_POLLING)
> -			ata_qc_set_polling(qc);
> +			ap->ops->freeze(ap);
>  
>  		ata_tf_to_host(ap, &qc->tf);

Wouldn't ata_tf_load() change the ctl value to qc->tf.ctl when issuing
the command?

I'm not really sure about this change because most controller which have
dedicated IRQ mask bit also have dedicated IRQ pending bit (and probably
IRQ clear bit too).  With those bits, IRQ during polling is not a big
problem (sata_sil for example).  So, the change doesn't really help the
controllers which actually have problems dealing with devices which
raise IRQ regardless of ATA_NIEN.

I don't see any easy way out other than manipulating host IRQ on/off as
IDE does.  If we choose to go that way, we can use them in
ata_bmdma_freeze/thaw (they should be named ata_sff_freeze/thaw really)
and use this patch on top of it.  Maybe it's better to drop ->freeze()
and ->thaw() altogether and use ->irq_on() and ->irq_off() instead.
Their names are more intuitive IMHO but that's minor.

Jeff, Alan, what do you think about adopting IDE's irq off/on mechanism.
 It's ugly and may cause some problems if the IRQ is shared but then
again it has been used for a long time without too much problem and
occasional lost IRQ is much better than dead machine due to screaming
IRQs followed by nobody cared.

Thanks.

-- 
tejun