Re: [RFC] pata_icside driver

Russell King
2007-04-09T11:33:52Z

On Mon, Apr 09, 2007 at 11:25:06AM +0100, Alan Cox wrote:
> > When you quantise those figures using a clock period of 62.5ns (16MHz)
> > you end up with: 2 clocks active (2*62.5 > 70), 1 clock recovery
> > (1*62.5 > 25) and 2 clocks cycle (2*62.5 > 120).
> >
> > Last time I checked, active + recovery must always be equal to the cycle
> > time, and unless my math is failing me, 2 + 1 does not equal 2.
>
> The libata code does the following:
>
> if (t->active + t->recover < t->cycle) {
> t->active += (t->cycle - (t->active + t->recover)) / 2;
> t->recover = t->cycle - t->active;
> }
>
> which probably means for 16MHz you don't have enough resolution to be sure
> you'll error in this direction in all cases. If so you just need to try
> adding
>
> if (t->active + t->recover > t->cycle)
> t->cycle = t->active + t->recover
>
> to stretch the cycle time to fit the resolution as well.
>
> And we should get this tested/fixed by someone seeing the problem, before
> and instead of putting hacks/notes in drivers that may then get lost

I agree; this is why I sent the patch out as an RFC before wanting it
merged. My feeling at the time when I wrote this is it's something that
the libata timing calculation code should already be catering for.

I should've explicitly mentioned that in the comments, but forgot.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/