linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Vitaly Bordug <vitb@kernel.crashing.org>
Cc: linuxppc-dev <linuxppc-dev@ozlabs.org>, Stefan Roese <sr@denx.de>
Subject: Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts
Date: Sat, 25 Aug 2007 11:49:58 +0200	[thread overview]
Message-ID: <67f53d72d17c98f2b8a76ec42195c0aa@kernel.crashing.org> (raw)
In-Reply-To: <20070825092954.4087.95333.stgit@localhost.localdomain>

> +		pci {
> +    			reg = <1 eec00000 40 1 ef400000 40>;  /* phb cfg, phb reg */

First component of reg is the unit address, so: pci@1eec00000 .

"phb cfg" is how you access PCI configuration space?  It wouldn't
hurt to document that, either in a little binding or just here in
the code.

> +			bus-range = <0 0>;

Can't you have subordinate PCI busses?  Or are there no slots :-)

> +			/*
> +			 * mem is at 80000000 set up indirectly
> +			 * io is at 0001_e800_0000
> +			 */
> +			ranges = <02000000 0 80000000 1 80000000 0 10000000
> +				01000000 0 00000000 1 e8000000 0 00100000>;

Comment doesn't match code for the memory space.  What does "set
up indirectly" mean here?  Oh wait, you want to say that the host
addresses 1_8000_0000..1_8fff_ffff are translated to PCI addresses
8000_0000..8fff_ffff.

What about PCI DMA, is that identity mapped?

> +			#interrupt-cells = <1>;
> +			#size-cells = <2>;
> +			#address-cells = <3>;

The reverse order of these is more conventional.  Not that it
actually matters ;-)

> +			compatible = "ibm, 440epx";

Stray space.  And you need to say it is the PCI host, so something
like "ibm,440epx-pci".


Segher

  reply	other threads:[~2007-08-25  9:50 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-25  9:29 [PATCH 0/3][POWERPC] Add PCI support for 44xEPx Vitaly Bordug
2007-08-25  9:29 ` [PATCH 1/3] [POWERPC] Merge 32 and 64 bit pci_process_bridge_OF_ranges() instances Vitaly Bordug
2007-08-27  1:15   ` David Gibson
2007-08-27  6:31     ` Vitaly Bordug
2007-08-27  7:49       ` David Gibson
2007-08-27  8:31         ` Vitaly Bordug
2007-08-25  9:29 ` [PATCH 2/3] [POWERPC] Add pci node to sequoia dts Vitaly Bordug
2007-08-25  9:49   ` Segher Boessenkool [this message]
2007-08-26 10:27     ` Vitaly Bordug
2007-08-26 19:10       ` Segher Boessenkool
2007-08-27  1:55       ` David Gibson
2007-08-25  9:51   ` Segher Boessenkool
2007-08-27  5:56     ` Stefan Roese
2007-08-27  1:54   ` David Gibson
2007-08-27  6:07     ` Stefan Roese
2007-08-27  6:21       ` David Gibson
2007-08-27  6:38         ` Stefan Roese
2007-08-27  6:50     ` Vitaly Bordug
2007-08-25  9:30 ` [PATCH 3/3] [POWERPC] Add PCI support for AMCC 440EPx (sequoia) Vitaly Bordug
2007-08-27  1:57   ` David Gibson
2007-08-27  6:21     ` Stefan Roese
2007-08-27 17:22       ` Josh Boyer
2007-08-28  0:21       ` David Gibson
2007-08-27  6:55     ` Vitaly Bordug
2007-08-27  8:05       ` Stefan Roese
2007-09-05 17:28   ` Valentine Barshak

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=67f53d72d17c98f2b8a76ec42195c0aa@kernel.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=sr@denx.de \
    --cc=vitb@kernel.crashing.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).