From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anton Vorontsov Subject: Re: [PATCH 2/3] [libata] pata_of_platform: OF-Platform PATA device driver Date: Thu, 29 Nov 2007 03:54:40 +0300 Message-ID: <20071129005440.GA2235@zarina> References: <20071127153708.GA12490@localhost.localdomain> <20071127153908.GB14183@localhost.localdomain> <20071127212222.GB10829@lixom.net> <200711272233.00456.arnd@arndb.de> <20071128154904.GA20722@localhost.localdomain> <474D9327.9020305@ru.mvista.com> Reply-To: cbou@mail.ru Mime-Version: 1.0 Content-Type: text/plain; charset=utf8 Return-path: Received: from mx34.mail.ru ([194.67.23.200]:8359 "EHLO mx34.mail.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758286AbXK2BBQ (ORCPT ); Wed, 28 Nov 2007 20:01:16 -0500 Content-Disposition: inline In-Reply-To: <474D9327.9020305@ru.mvista.com> Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Sergei Shtylyov Cc: avorontsov@ru.mvista.com, Olof Johansson , linuxppc-dev@ozlabs.org, Arnd Bergmann , linux-ide@vger.kernel.org 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@mail.ru backup email: ya-cbou@yandex.ru irc://irc.freenode.net/bd2