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
pgpRHmTjNPrYI.pgp
Description: PGP signature
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev