From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTP id 4B9A0DDF13 for ; Wed, 12 Sep 2007 01:55:59 +1000 (EST) In-Reply-To: <20070911141711.GE1932@ld0162-tx32.am.freescale.net> References: <20070911141711.GE1932@ld0162-tx32.am.freescale.net> Mime-Version: 1.0 (Apple Message framework v752.2) Content-Type: text/plain; charset=US-ASCII; delsp=yes; format=flowed Message-Id: <1DE5CB62-9EF1-42BA-93F3-CE15DD94F5DD@kernel.crashing.org> From: Kumar Gala Subject: Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port Date: Tue, 11 Sep 2007 10:58:43 -0500 To: Scott Wood Cc: linuxppc-dev@ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Sep 11, 2007, at 9:17 AM, Scott Wood wrote: > On Tue, Sep 11, 2007 at 01:29:18AM -0500, Kumar Gala wrote: >> diff --git a/arch/powerpc/boot/dts/mpc8572ds.dts b/arch/powerpc/ >> boot/dts/mpc8572ds.dts >> new file mode 100644 >> index 0000000..9d23561 >> --- /dev/null >> +++ b/arch/powerpc/boot/dts/mpc8572ds.dts >> @@ -0,0 +1,378 @@ >> +/* >> + * MPC8572 DS Device Tree Source >> + * >> + * Copyright 2007 Freescale Semiconductor Inc. >> + * >> + * This program is free software; you can redistribute it and/or >> modify it >> + * under the terms of the GNU General Public License as >> published by the >> + * Free Software Foundation; either version 2 of the License, >> or (at your >> + * option) any later version. >> + */ >> + >> +/ { >> + model = "MPC8572DS"; >> + compatible = "MPC8572DS", "MPC85xxDS"; > > "fsl," prefix on compatible. > >> + #address-cells = <1>; >> + #size-cells = <1>; >> + >> + cpus { >> + #cpus = <1>; > > Where is #cpus defined or used? I don't see it used in any other > trees, either. will kill. > >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + PowerPC,8572@0 { >> + device_type = "cpu"; >> + reg = <0>; >> + d-cache-line-size = <20>; // 32 bytes >> + i-cache-line-size = <20>; // 32 bytes >> + d-cache-size = <8000>; // L1, 32K >> + i-cache-size = <8000>; // L1, 32K >> + timebase-frequency = <0>; >> + bus-frequency = <0>; >> + clock-frequency = <0>; >> + 32-bit; > > Where is 32-bit; defined or used? its a copy paste error. Will kill 32-bit on other platforms. > >> + soc8572@ffe00000 { >> + #address-cells = <1>; >> + #size-cells = <1>; >> + #interrupt-cells = <2>; >> + device_type = "soc"; >> + ranges = <00000000 ffe00000 00100000 >> + 80000000 80000000 20000000 >> + a0000000 a0000000 20000000 >> + c0000000 c0000000 20000000 >> + ffc00000 ffc00000 00010000 >> + ffc10000 ffc10000 00010000 >> + ffc20000 ffc20000 00010000>; >> + >> + reg = ; // CCSRBAR 1M > > Comment doesn't match what's actually in reg. will fix. > >> + bus-frequency = <0>; // Filled out by uboot. >> + >> + memory-controller@2000 { >> + compatible = "fsl,8572-memory-controller"; > > Is it compatible with any other 85xx memory controller? maybe, but I don't want to get into that just yet. > >> + reg = <2000 1000>; >> + interrupt-parent = <&mpic>; >> + interrupts = <12 2>; >> + }; >> + >> + l2-cache-controller@20000 { >> + compatible = "fsl,8572-l2-cache-controller"; >> + reg = <20000 1000>; >> + cache-line-size = <20>; // 32 bytes >> + cache-size = <80000>; // L2, 512K >> + interrupt-parent = <&mpic>; >> + interrupts = <10 2>; >> + }; > > Should this node be referenced by an l2-cache property in the cpu > node? No, its a front side cache. > >> + pcie@8000 { >> + compatible = "fsl,mpc8641-pcie"; > > Should probably be "fsl,mpc8572-pcie", "fsl,mpc8641-pcie". this should "fsl,mpc8548-pcie" to match other 85xx pcie. > >> + device_type = "pci"; >> + #interrupt-cells = <1>; >> + #size-cells = <2>; >> + #address-cells = <3>; >> + reg = <8000 1000>; >> + bus-range = <0 ff>; >> + ranges = <02000000 0 80000000 80000000 0 20000000 >> + 01000000 0 00000000 ffc00000 0 00010000>; > > No prefetchable mem space? we haven't normally provided prefetch on 85xx/86xx.. will deal with this later. > >> + clock-frequency = <1fca055>; > > Decimal would be nicer. > >> + pcie@9000 { >> + compatible = "fsl,mpc8548-pcie"; > > Why is this one 8548-compatible and the previous one 8641-compatible? copy paste, will fix. > >> + mpic: pic@40000 { >> + clock-frequency = <0>; >> + interrupt-controller; >> + #address-cells = <0>; >> + #interrupt-cells = <2>; >> + reg = <40000 40000>; >> + built-in; >> + compatible = "chrp,open-pic"; >> + device_type = "open-pic"; >> + big-endian; >> + }; > > Where is the built-in; property defined or used? defined in chrp OF spec, not sure why we bother with it. > >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h >> index 06d23e1..c98b867 100644 >> --- a/include/linux/pci_ids.h >> +++ b/include/linux/pci_ids.h >> @@ -374,10 +374,9 @@ >> #define PCI_DEVICE_ID_ATI_IXP400_SATA 0x4379 >> #define PCI_DEVICE_ID_ATI_IXP400_SATA2 0x437a >> #define PCI_DEVICE_ID_ATI_IXP600_SATA 0x4380 >> -#define PCI_DEVICE_ID_ATI_IXP600_SMBUS 0x4385 >> +#define PCI_DEVICE_ID_ATI_SBX00_SMBUS 0x4385 >> #define PCI_DEVICE_ID_ATI_IXP600_IDE 0x438c >> #define PCI_DEVICE_ID_ATI_IXP700_SATA 0x4390 >> -#define PCI_DEVICE_ID_ATI_IXP700_SMBUS 0x4395 >> #define PCI_DEVICE_ID_ATI_IXP700_IDE 0x439c > > What's going on here? copied file over from an older tree. - k