Re: pata_cs5536: ATA driver for Geode companion chip

["Jordan Crouse" <jordan.crouse@amd.com>]

On 03/10/07 13:28 -0400, Martin K. Petersen wrote:
> 
> This is a driver specifically for the ATA controller on the Geode
> CS5536 companion chip.  The PCI device ID for this device was
> previously claimed by pata_amd.c but there were two problems with
> that:
 
Hmm - I had to ruminate on this for a while.   As a general rule of thumb
I try to discourage working around the PCI virtualization - both because
it encumbers Linux with one off drivers such as this one, and we lose
any benefit from bug fixes in the firmware (not to mention bug fixes in
the common shared driver).  AMD has gone to a lot of trouble and expense
to make a decidedly non-PCI bus Just Work (TM) like a PCI bus, and it
takes a lot to convince me to diverge from that.

>  - Not all Geode platforms emulate the ATA registers in PCI config
>    space

If you system has any sort of PCI virtualization support at all,
and it doesn't support these registers, then thats a big fat
bug, IMHO, and should be resolved post haste.  But that said, 
the VSA doesn't do any sort of messy workaround here, so the direct
route isn't going to lose us anything in terms of bug fixes.

>  - CS5536 PIO timings are not identical to AMD74XX/8111

This is true, and it was brought up when we first pushed the drivers.
Tests showed that the timings generated by the Thor driver were within
the optimal tolerances for the 5536 controller, and we decided that a
custom driver wasn't indicated.  But if we *are* going to have a custom
driver, then we should get these right.

> pata_cs5536.c relies on Geode Machine Specific Registers to configure
> the ATA function and uses the correct PIO timings for the chip.

I have to discourage this patch just on principle, because I don't like
the idea of working around the VSA.  That said, it seems that Alan is 
willing to take on the extra code, and there isn't anything technically
deficient here, so I'm fine with this going in.

Jordan