public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Sergei Shtylyov <sshtylyov@ru•mvista.com>
To: avorontsov@ru•mvista.com
Cc: linuxppc-dev@ozlabs•org, linux-ide@vger•kernel.org
Subject: Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
Date: Tue, 27 Nov 2007 19:46:13 +0300	[thread overview]
Message-ID: <474C49D5.2000206@ru.mvista.com> (raw)
In-Reply-To: <20071127164101.GA14790@localhost.localdomain>

Anton Vorontsov wrote:

>>>This patch adds localbus and pata nodes to use CF IDE interface
>>>on MPC8349E-mITX boards.

>>>Patch also adds code to probe localbus.

>>>Signed-off-by: Anton Vorontsov <avorontsov@ru•mvista.com>
>>>---
>>>arch/powerpc/boot/dts/mpc8349emitx.dts    |   17 ++++++++++++++++-
>>>arch/powerpc/platforms/83xx/mpc834x_itx.c |   17 +++++++++++++++++
>>>2 files changed, 33 insertions(+), 1 deletions(-)

>>>diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts 
>>>b/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>index 5072f6d..7a97068 100644
>>>--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>@@ -249,6 +249,21 @@
>>>		device_type = "pci";
>>>	};
>>>
>>>+	localbus@e0005000 {
>>>+		#address-cells = <2>;
>>>+		#size-cells = <1>;
>>>+		compatible = "fsl,mpc8349emitx-localbus",
>>
>>   Board compatible bus?

> This is what Documentation/powerpc/booting-without-of.txt suggests
> for localbuses. I'm following.

    Hm...

>>>+			     "fsl,mpc8349e-localbus",
>>>+			     "fsl,pq2pro-localbus";
>>>+		reg = <e0005000 d8>;
>>>+		ranges = <3 0 f0000000 210>;
>>>
>>>-
>>>+		pata@3,0 {
>>>+			compatible = "fsl,mpc8349emitx-pata", 
>>>"pata-platform";
>>>+			reg = <3 0 10 3 20c 4>;
>>>+			ioport-shift = <1>;

>>   Bleh... that shift again. And this is surely not a good name for a 
>>property (where's I/O ports in your case?) -- why not call it "reg-shift" 
>>(well, I'd call it "reg-size" or "reg-stride" myself :-)?

> 1. "shift" because pata_platform using that name. I don't see any
>    reason to contrive indirections. ioport-shift is what the whole
>    Linux kernel using nowadays, and ioport-shift dts property
>    anyway Linux-specific.

    It's just a bad name. There's not even I/O ports in this case (and 
moreover, the *real* I/O mapped device would always have a shift of 0, I bet 
-- larger strides are for memory mapped devices).

>    I'm just following todays' conventions.

>    If you feel really bad about that, I think better to fix that in
>    the source of the badness -- pata_platform. It's easy, I can do

    I only feel really bad about the "ioport" part, I can live with "shift" 
part. :-)

>    that. Would you ack patch that converts whole pata_platform and
>    users? Would Paul ack it?

    I don't understand -- why the property name should duplicate pata_platform 
field name? :-O

>    Still, is there any hardware that needs not power of 2 stride?

    Not really -- "size" just seems better, aesthetically. :-)

> 2. "ioport" because shift^Wstride ;-) applies only to the io range
>    (yes, it's obvious, but worth open-wording, no?).

    Contrarywise, to memory range.

> And btw, I can get rid of ioport-shift at all. And do fixups in
> the pata_of_platform driver via .compatible matching. But I don't
> want: it feels bad to list every needs-to-fixup board in the common
> driver. It also feels not so great creating something like
> pata-platform-stride-{1,2,4,...} compatible stuff. Heh.

    I didn't propose neither of that. :-)
    All I want is that "ioport-*" be renamed.

> Thanks,

MBR, Sergei

  reply	other threads:[~2007-11-27 17:18 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-27 15:37 [PATCH 0/3] OF-platform PATA driver Anton Vorontsov
2007-11-27 15:39 ` [PATCH 1/3] [libata] pata_platform: make probe and remove functions device type neutral Anton Vorontsov
2007-11-27 22:31   ` Arnd Bergmann
2007-11-27 15:39 ` [PATCH 2/3] [libata] pata_of_platform: OF-Platform PATA device driver Anton Vorontsov
2007-11-27 21:22   ` Olof Johansson
2007-11-27 21:32     ` Arnd Bergmann
2007-11-28 15:49       ` Anton Vorontsov
2007-11-28 16:11         ` Sergei Shtylyov
2007-11-29  0:54           ` Anton Vorontsov
2007-11-30 10:17             ` Sergei Shtylyov
2007-11-30 10:58               ` Anton Vorontsov
2007-11-30 11:05                 ` Sergei Shtylyov
2007-11-30 11:45                   ` Anton Vorontsov
2007-11-30 11:43                 ` Sergei Shtylyov
2007-11-30 12:09                   ` Anton Vorontsov
2007-11-28 16:29       ` Sergei Shtylyov
2007-11-28  0:41   ` Stephen Rothwell
2007-12-02  3:59   ` Olof Johansson
2007-11-27 15:39 ` [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes Anton Vorontsov
2007-11-27 15:49   ` Sergei Shtylyov
2007-11-27 16:41     ` Anton Vorontsov
2007-11-27 16:46       ` Sergei Shtylyov [this message]
2007-11-27 17:27         ` Anton Vorontsov
2007-11-27 17:25           ` Sergei Shtylyov
2007-11-27 17:34         ` Anton Vorontsov
2007-11-27 17:34           ` Sergei Shtylyov
2007-11-27 17:48             ` Anton Vorontsov
2007-11-27 18:07               ` Sergei Shtylyov
2007-11-27 19:50                 ` Anton Vorontsov
2007-11-27 21:18     ` Kumar Gala
2007-11-28  9:51 ` [PATCH 0/3] OF-platform PATA driver Paul Mundt
2007-11-28 13:24   ` Anton Vorontsov
2007-12-01 22:54 ` Jeff Garzik
2007-12-01 23:58   ` Anton Vorontsov
2007-12-02  3:57     ` Olof Johansson
2007-12-02 11:46       ` Anton Vorontsov
2007-12-02 15:45         ` Olof Johansson
2007-12-02 23:40           ` Arnd Bergmann
2007-12-02 23:58             ` Olof Johansson
  -- strict thread matches above, loose matches on Subject: below --
2007-12-14 18:21 [PATCH v3 " Anton Vorontsov
2007-12-14 18:25 ` [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes Anton Vorontsov
2008-01-09 19:08 [PATCH v4 0/3] OF-platform PATA driver Anton Vorontsov
2008-01-09 19:11 ` [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes Anton Vorontsov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=474C49D5.2000206@ru.mvista.com \
    --to=sshtylyov@ru$(echo .)mvista.com \
    --cc=avorontsov@ru$(echo .)mvista.com \
    --cc=linux-ide@vger$(echo .)kernel.org \
    --cc=linuxppc-dev@ozlabs$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox