On Wed, Nov 28, 2007 at 07:11:19PM +0300, Sergei Shtylyov wrote:

Anton Vorontsov wrote:

quoted

quoted

quoted

quoted

This driver nicely wraps around pata_platform library functions,
and provides OF platform bus bindings to the PATA devices.

quoted

quoted

quoted

quoted

+static struct of_device_id pata_of_platform_match[] = {
+     { .compatible = "pata-platform", },
+};

quoted

quoted

quoted

"pata-platform" really means nothing outside of linux. A more
generic label would be useful.

quoted

Agreed.

    Now you're quick to agree. :-)

I'm quick to change my mind either. ;-)

*BOOM*, I changed my mind.

quoted

quoted

Maybe the name of the standards it supports? Could be
"ata-4", "ata-5" and the like, or the exact transfer mode, like
"pata-udma-5", "pata-pio-3", "sata-150", etc.

quoted

You're quite optimistic about pata_platform capabilities. ;-)

    Indeed. :-)

quoted

As far as I know it is [obviously] supports PIO modes only. And so
far I was able to get max 5.28 MB/s read transfers. Which looks like
ideal case for PIO1 (CF I'm testing on is 3.0, max. PIO4).

    Believe me, it's a great speed even for PIO4. Most systems only show 3+ 
MiB/s in this mode according to hdparm.

quoted

I've modified pio_mask appropriately, plus I've tried to comment
out .set_mode = pata_platform_set_mode, and now it says:

quoted

ata5: PATA max PIO4 mmio cmd 0xf0000000 ctl 0xf000020c irq 24
ata5.00: CFA: TOSHIBA THNCF512MQG, 3.00, max PIO4
ata5.00: configured for PIO4
ata5.00: configured for PIO4

quoted

That looks good, but speed is the same. Oh well, it's another
matter.

quoted

Back to dts, I think pata-pio-X is good scheme. That way we can
pass pio_mask via device tree. Sounds reasonable?

    Grumble. Can't we pass this via some property other than "compatible"? I'm 
opposed to "ata-5" and the like in there as well cause it's not clear what 
information this would provide. Why people so love to make things complex WRT 
the "compatible" property -- instead of making the task of selecting a proper 
driver more simple, they tend to make it mode complex by trying to specify 
values that have quite little to do with the device's programming interface 
itself...

Ok, now I'm agree here. dts already specifying "fsl,mpc8349emitx-pata",
second compatible entry is okay to mean nothing outside Linux itself,
there are plenty of examples for such kind.

Remaining question: any preferred name for that property? pio-mode okay?
It's assuming that PIO6 capable bus supports PIO0 as well, thus no mask.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2