Re: sata_nv + ADMA + Samsung disk problem

[Mark Lord <liml@rtr.ca>]

Mark Lord wrote:
> Robert Hancock wrote:
>> Mark Lord wrote:
>>> Robert Hancock wrote:
>>> ..
>>>>  From some of the traces I took previously (posted on LKML as "sata_nv ADMA controller lockup investigation" way back in Feb 07), what seems to occur is that when the second command is issued very rapidly (within less than 20 microseconds, or potentially longer) after the previous command's completion, the ADMA status changes from 0x500 (STOPPED and IDLE) to 0x400 (just IDLE) as it typically does, but then it sticks there, no interrupt is ever raised, and CPB response flags remain at 0.
>>> ..
>>>
>>> Assuming that NVidia got their ADMA core logic from Pacific Digital
>>> (the inventors), then it may have some of the same bugs as the original.
>>>
>>> One of those bugs is that the aGO trigger is sampled in a "racey" way,
>>> such that it sometimes may miss a recent addition to the ring.
>>>
>>> The *only* way to guarantee things with the original Pacific Digital core
>>> was to (1) always retrigger aGO for a full ring scan with each new addition,
>>> and (2) poll periodically (every half second or so) rather than relying
>>> exclusively on the IRQ actually working..
>>>
>>> Dunno about the NVidia version.
>>
>> Theirs works rather differently - the GO bit is there, but there's another append register which is used to tell the controller that a new tag has been added to the CPB list.
> ..
> 
> The PacDigi core uses a "search count" register for that purpose,
> but the buggy nature of the core required that it always be set
> to "2 * ring_size" to ensure nothing got missed.
> 
> Here's some comments from the original ADMA driver.
> Maybe something from here might help with the NV stuff, too.
> 
>       // There is a chance that the chip will skip over a CPB if a SERVICE interrupt
>        // occurs while it's reading the CPB header.  This won't cause us to get
>        // stuck anywhere, but it might slow down execution of the new CPB if
>        // it has to wait for the next time we hit aGO.  So.. Dxxx/Dxxx suggest
>        // that all we need to do is tell the chip to do two passes around the ring
>        // from an aGO instead of one pass, so that it will find the "missed" CPB
>        // on the second pass.  This isn't as bad as it first looks.
>        //
>        writew(channel->num_cpbs * 2, &adma_regs->cpb_search_count);
> 
> Or again, the NV stuff may be completely different (?).
..

Another thing about the PacDigi core:  one has to be very careful
to avoid sequential accesses to sequential PCI locations when
programming the chip -- it cannot handle merged register writes.

So for any group of sequentially laid out registers, the code has
to ensure it never writes two adjacent registers in sequence..

-ml