Re: [PATCH] libata: disable_irq() during polling IDENTIFY

[Tejun Heo <htejun@gmail.com>]

Hello, Albert.

Albert Lee wrote:
> Tejun Heo wrote:
>> Also, this is a problem for not only IDENTIFY but all polling commands.
> 
> Yes, other command might also assert INTRQ during polling.
> However, for the specific BENQ DW1620 drive, only IDENTIFY_PACKET_DEVICE
> has such behavior; other commands like READ or REQUEST_SENSE are ok.

Oh I see.

>> One solution I can think of is to let IRQ handler ack IRQ
>> unconditionally during polling commands - ie. just read the TF Status
>> register once and tell the IRQ subsystem that the IRQ is handled.  This
>> shouldn't affect the operation of polling as the only side effect of
>> reading Status is clearing pending IRQ && will give us a nice way to
>> deal with the SATA bridge chip which chokes on nIEN.  Considering the
>> sorry state of nIEN in SATA, I guess this might be the best way to deal
>> with this.
>>
>> Albert, can you please test whether this works?  Modifying
>> ata_interrupt() such that it reads TF Status if ATA_TFLAG_POLLING should
>> do the trick.
> 
> Yes, reading the Status register and acking interrupt also fixes the
> problem (patch attached below). 

Good to know it works.  With or without nIEN, I think this change is a
good thing to have.  IRQ handler shouldn't interfere with polling as
both acquire lock during operation.  Polling is safer this way and
non-SFF drivers are doing something similar anyway.  I also think it
would be nice to have the SFF irq_handler clear IRQ while the port is
frozen.  This again is the default behavior of non-SFF drivers.

Any objections?

-- 
tejun