On Wed, Nov 28, 2007 at 07:11:19PM +0300, Sergei Shtylyov wrote:
> Anton Vorontsov wrote:
>
> >>>>This driver nicely wraps around pata_platform library functions,
> >>>>and provides OF platform bus bindings to the PATA devices.
>
> >>>>+static struct of_device_id pata_of_platform_match[] = {
> >>>>+ { .compatible = "pata-platform", },
> >>>>+};
>
> >>>"pata-platform" really means nothing outside of linux. A more
> >>>generic label would be useful.
>
> > Agreed.
>
> Now you're quick to agree. :-)
I'm quick to change my mind either. ;-)
*BOOM*, I changed my mind.
> >>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.
>
> > You're quite optimistic about pata_platform capabilities. ;-)
>
> Indeed. :-)
>
> > 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.
>
> > I've modified pio_mask appropriately, plus I've tried to comment
> > out .set_mode = pata_platform_set_mode, and now it says:
>
> > 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
>
> > That looks good, but speed is the same. Oh well, it's another
> > matter.
>
> > 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@xxxxxxx
backup email: ya-cbou@xxxxxxxxx
irc://irc.freenode.net/bd2
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
