Re: [PATCH] Serialize CMD643 and CMD646 to fix a hardware bug with SSD

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

Hello.

Mikulas Patocka wrote:

>> Right, and see also:
>>
>> commit 6b5cde3629701258004b94cde75dd1089b556b02
>> Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>> Date:   Mon Dec 29 20:27:32 2008 +0100
>>
>>     cmd64x: set IDE_HFLAG_SERIALIZE explictly for CMD646
>>
>> Which is how we got there.
>>
>> The most conservative thing to do is to set the flag as
>> is done by mpatocka's patch but I'd like Frans to regression
>> test that on his ultra5.
>>
>> Frans can you do that?
>>
>> Thanks.
>>     
>
> I read the thread about wild interrupts that Frans was observing and that 
> led to disabling the serialization.
>
> The thing is tricky --- if we read status register on interrupt, we break 
> the serialization principle and introduce potential data corruption.
>
> If we don't read the status register, the wild interrupt kills the whole 
> interrupt line.
>
> I think the interrupt handler should read the BM-status register on both 
> channels (it reflects the interrupt state even in PIO mode) to test if 
>   

   Are you sure? Anyway, there's no need as we're reading the interrupt 
bits CFR/ARTTIM23 registers first (at least in the IDE code). Look at 
the cmd*_test_irq() methods and ide_intr().

> there is an unexpected interrupt on the inactive channel --- this should 
> be much more safe than reading the status register. If there is an 
> interrupt, then
> --- read the status of the inactive channel? (potential data corruption, 
> but it is reported to happen only on boot).
> --- Or can the interrupt be acknowledged only in BM-status without 
> touching the device? I believe yes,

   And I believe no. BMIDE status bit doesn't acknoledge (clear, to be 
precise) the IDE interrupts, only the status register read does.


> it shoud shut the PCI interrupt but it 
> would leave the IDE interrupt line on (should be cleared on next command).
>   

    I think the negated wired-OR of both INTRQ signals serves as an 
-INTA source, not the BMIDE status bits. At least in the general case, 
where the BMIDE status doesn't reflect PIO mode interrupts.

> Mikulas

WBR, Sergei