From: Tony Breeds <tony@bakeyournoodle•com>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>
Cc: LinuxPPC-dev <linuxppc-dev@lists•ozlabs.org>
Subject: Re: [PATCH 6/6] 44x/currituck: Add support for the new IBM currituck platform
Date: Thu, 1 Dec 2011 15:05:24 +1100 [thread overview]
Message-ID: <20111201040524.GF15560@thor.bakeyournoodle.com> (raw)
In-Reply-To: <1322634022.21641.61.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 4483 bytes --]
On Wed, Nov 30, 2011 at 05:20:22PM +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2011-11-30 at 16:24 +1100, Tony Breeds wrote:
> > + plb {
> > + compatible = "ibm,plb-4xx", "ibm,plb4"; /* Could be PLB6, doesn't matter */
>
> Then make it plb6 and add it to the probe list. Might have to whack it's
> configuration registers one day etc...
Done.
> > + * XXX: 1 TB address space, do we really care past
> > + * 4 GB and should we expand cell width?
> > + */
>
> For OPB probably not :-)
Fixed.
> That doesn't seem like a very useful pair of statements or useful debug
> message....
All the DBG cruft is gone.
> > + for(i = 0; i < MAX_RANKS; i++){
> > + reg = mfdcrx(DDR3_MR0CF + i);
> > + printf("%s: reg=0x%08x\r\n", __func__, reg);
>
> All that debug is pretty gross, keep it if you wish but make it a bit
> neater and/or wrap it in DBG
Yeah that was an oversight.
> > + if (reg & 0x01) {
>
> if (!(reg & 1))
> continue;
>
> avoids too much indent
Done.
> > + dt_fixup_memory(0x0ULL, ibm_currituck_memsize);
>
> Ok, I see why the global... I'd still prefer if the detect function just
> returned the value and the caller whacks the global.
Fixed.
> > + while ((devp = find_node_by_devtype(devp, "pci"))) {
> > + if (getprop(devp, "dma-ranges", &dma_ranges[0], sizeof(dma_ranges)) < 0) {
>
> Can't you replace &dma_ranges[0] with just dma_ranges ?
Yes, Fixed.
> > +#define SPRN_PIR 0x11E /* Processor Indentification Register */
>
> That should go elsewhere along with the other SPR definitions.
There isn't a std. place in the bootwrapper.
>
> > +void platform_init(void)
> > +{
> > + /* Cap the zImage to 512MB */
>
> Any reason ? If yes, please document it a bit more.
XXX
> > + node = fdt_node_offset_by_prop_value(_dtb_start, -1, "device_type",
> > + "cpu", sizeof("cpu"));
> > + if (!node)
> > + fatal("Cannot find cpu node\n");
>
> The above will return -a- CPU node... you have several and you don't
> know which one. You should probably iterate accross all of them.
I'm just trying to get the timebase-frequency, this willbe the same on
all CPUs (at least I hope so ;P) so I don't really care which CPU node I
get.
> > + /* FIXME: Check this works */
Grr that comment shouldn't be there. It does work :)
> > +#define PVR_476CURRITUCK 0x7ff50000
>
> My understanding is that the currituck was the platform, not the chip,
> and that the chip was called something like 476FPE, am I wrong ?
No you're correct, I was confused about the boundry between 476fpe and
currituck.
> > + cmplwi cr0,r3,PVR_476CURRITUCK@h
> > + beq head_start_47x
>
> So at some point, they gave us the magic foo to do with the PVR to
> identify any 476... I'll try to dig that out of my email archives.
Yeah I have that I'll work out the correct way to the the mask and
test.
> > +++ b/arch/powerpc/platforms/44x/ppc47x.c
>
> Call the file currituck.c
Sure.
> > +static void __devinit quirk_ppc_currituck_usb_fixup(struct pci_dev *dev)
> > +{
> > + pci_write_config_dword(dev, 0xe0, 0x0114231f);
> > + pci_write_config_dword(dev, 0xe4, 0x00006c40);
> > +}
>
> Pleae document better what you are doing here and also test
> that you are indeed on the right platform so you don't end up
> whacking bits on USB controllers on other platforms that happen
> to be compiled in the same binary.
Sure, no problem.
> Ok, I'll have to fixup that vs. Kyle patches but the good thing is that
> it will make things even simpler.
Thanks.
> Now pretty much everything in this platform file is generic I believe.
> We could move it all to ppc44x_simple.c. The only things that are not
> are the USB quirk and the interrupt fixup.
>
> The USB quirk which should have a compatible test for the platform to
> make sure we don't run it on something else. For such a simple quirk, I
> think it's fine ot have it in ppc44x_simple.c or in drivers/pci/quirk.c
>
> For the interrupt fixup, we can probably address it entirely in the
> device-tree, though that means exposing a bunch of on-board bridges
> which is only midly annoying.
>
> Anyways, we can discuss that (or maybe an even better option)
> tomorrow :-)
Okay I look forward to it :)
> Again, you are mixing the SoC with the board here. afaik, currituck is
> the board, not the SoC.
Fixed.
Yours Tony
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
next prev parent reply other threads:[~2011-12-01 4:05 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-30 5:23 Add support for the currituck 476 platform from IBM Tony Breeds
2011-11-30 5:23 ` [PATCH 1/6] 44x/pci: Continue pci setup even if there is no sdr-base in the device-tree Tony Breeds
2011-11-30 5:46 ` Benjamin Herrenschmidt
2011-12-01 1:11 ` Tony Breeds
2011-11-30 5:23 ` [PATCH 2/6] 44x/pci: Setup the dma_window properties for each pci_controller Tony Breeds
2011-11-30 5:23 ` [PATCH 3/6] 44x: Removing dead CONFIG_PPC47x Tony Breeds
2011-11-30 11:43 ` Josh Boyer
2011-11-30 20:20 ` Benjamin Herrenschmidt
2011-11-30 5:23 ` [PATCH 4/6] powerpc/boot: Add extended precision shifts to the boot wrapper Tony Breeds
2011-11-30 5:48 ` Benjamin Herrenschmidt
2011-11-30 11:45 ` Josh Boyer
2011-11-30 20:21 ` Benjamin Herrenschmidt
2011-11-30 20:28 ` Scott Wood
2011-11-30 23:25 ` Tony Breeds
2011-11-30 5:23 ` [PATCH 5/6] powerpc/boot: Add mfdcrx Tony Breeds
2011-11-30 13:09 ` Segher Boessenkool
2011-11-30 18:10 ` David Laight
2011-12-01 22:55 ` Segher Boessenkool
2011-11-30 23:30 ` Tony Breeds
2011-11-30 23:35 ` Tony Breeds
2011-11-30 5:24 ` [PATCH 6/6] 44x/currituck: Add support for the new IBM currituck platform Tony Breeds
2011-11-30 6:20 ` Benjamin Herrenschmidt
2011-11-30 6:31 ` David Gibson
2011-12-01 4:05 ` Tony Breeds [this message]
2011-12-01 4:29 ` Tony Breeds
2011-11-30 13:23 ` Kumar Gala
2011-12-01 4:16 ` Tony Breeds
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=20111201040524.GF15560@thor.bakeyournoodle.com \
--to=tony@bakeyournoodle$(echo .)com \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.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