: [PATCH] Add_460SX_Initial_Framework

Josh,
  I will be handling this patch from now on. I will modify the patch and
answer your queries soon.
Thanks,
Marri


Message: 2
Date: Mon, 1 Dec 2008 20:32:56 -0500
From: Josh Boyer <redacted>
Subject: Re: [PATCH] Add_460SX_Initial_Framework
To: mmadishetty@amcc.com
Cc: linuxppc-dev@ozlabs.org
Message-ID: [ref]
Content-Type: text/plain; charset=3Dus-ascii

On Mon, Dec 01, 2008 at 03:37:15PM -0800, mmadishetty@amcc.com wrote:

From: Madhulika Madishetty <redacted>

This patch contains the initial framework for AMCC Redwood board.

Signed-off-by: Madhulika Madishetty <redacted>, Tirumala

Reddy=20

Marri [off-list ref],
Feng Kan [off-list ref], Vidhyananth Venkatasamy

[off-list ref],=20

Preetesh Parekh [off-list ref]
Acked-by: Loc Ho <redacted>, Feng Kan <redacted>

One Signed-off-by: per person, per line please.  Don't use a single
with multiple names.

---
arch/powerpc/boot/dts/redwood_amcc.dts     |  247 +++++++
arch/powerpc/configs/44x/redwood_defconfig | 1082=20
++++++++++++++++++++++++++++

Parts of your patch are word-wrapped.

Any particular reason you chose to call this redwood_amcc.dts?  None
of the other boards do that.

Also, what possessed AMCC to create an entirely new board called
Redwood when there is already a 4xx board called Redwood?  I realize
this isn't really something you can control, and the old board isn't
supported any longer, but still...  yell at your marketing people or
something :).

If this is really a dts-v1, I would expect all the values here to
look differently.  See below.

+
+/ {
+	#address-cells =3D <2>;
+	#size-cells =3D <1>;
+	model =3D "amcc,redwood";
+	compatible =3D "amcc,redwood";
+	dcr-parent =3D <&/cpus/cpu@0>;
+
+	aliases {
+		ethernet0 =3D &EMAC0;
+		serial0 =3D &UART0;
+	};
+
+	cpus {
+		#address-cells =3D <1>;
+		#size-cells =3D <0>;
+
+		cpu@0 {
+			device_type =3D "cpu";
+			model =3D "PowerPC,460SX";
+			reg =3D <0>;
+			clock-frequency =3D <0>; /* Filled in by U-Boot */
+			timebase-frequency =3D <0>; /* Filled in by U-Boot

*/

+			i-cache-line-size =3D <20>;
+			d-cache-line-size =3D <20>;

Here.  You have a i/d-cache line size of 20 bytes?  That's odd...

+			i-cache-size =3D <8000>;
+			d-cache-size =3D <8000>;

And you have a cache size of 8000 bytes?  Also odd.  I would expect
these
lines to look like:

			i-cache-line-size =3D <0x20>;
			i-cache-size =3D <0x8000>;

or
			i-cache-line-size =3D <32>;
			i-cache-size =3D <32768>;

Please go through and verify all the values are properly filled out.
I'm
not even sure how this works with newer dtc versions.

+			dcr-controller;
+			dcr-access-method =3D "native";
+		};
+	};
+
+	memory {
+		device_type =3D "memory";
+		reg =3D <0 0 0>; /* Filled in by U-Boot */
+	};
+
+	UIC0: interrupt-controller0 {
+		compatible =3D "ibm,uic-460sx","ibm,uic";
+		interrupt-controller;
+		cell-index =3D <0>;
+		dcr-reg =3D <0c0 009>;
+		#address-cells =3D <0>;
+		#size-cells =3D <0>;
+		#interrupt-cells =3D <2>;
+	};
+
+	UIC1: interrupt-controller1 {
+		compatible =3D "ibm,uic-460sx","ibm,uic";
+		interrupt-controller;
+		cell-index =3D <1>;
+		dcr-reg =3D <0d0 009>;
+		#address-cells =3D <0>;
+		#size-cells =3D <0>;
+		#interrupt-cells =3D <2>;
+		interrupts =3D <1e 4 1f 4>; /* cascade */
+		interrupt-parent =3D <&UIC0>;
+	};
+
+	UIC2: interrupt-controller2 {
+		compatible =3D "ibm,uic-460sx","ibm,uic";
+		interrupt-controller;
+		cell-index =3D <2>;
+		dcr-reg =3D <0e0 009>;
+		#address-cells =3D <0>;
+		#size-cells =3D <0>;
+		#interrupt-cells =3D <2>;
+		interrupts =3D <a 4 b 4>; /* cascade */
+		interrupt-parent =3D <&UIC0>;
+	};
+
+	UIC3: interrupt-controller3 {
+		compatible =3D "ibm,uic-460sx","ibm,uic";
+		interrupt-controller;
+		cell-index =3D <3>;
+		dcr-reg =3D <0f0 009>;
+		#address-cells =3D <0>;
+		#size-cells =3D <0>;
+		#interrupt-cells =3D <2>;
+		interrupts =3D <10 4 11 4>; /* cascade */
+		interrupt-parent =3D <&UIC0>;
+	};
+
+	SDR0: sdr {
+		compatible =3D "ibm,sdr-460sx";
+		dcr-reg =3D <00e 002>;
+	};
+
+	CPR0: cpr {
+		compatible =3D "ibm,cpr-460sx";
+		dcr-reg =3D <00c 002>;
+	};
+	plb {
+		compatible =3D "ibm,plb-460sx", "ibm,plb4";
+		#address-cells =3D <2>;
+		#size-cells =3D <1>;
+		ranges;
+		clock-frequency =3D <0>; /* Filled in by U-Boot */
+
+		SDRAM0: sdram {
+			compatible =3D "ibm,sdram-460sx",

"ibm,sdram-405gp";

+			dcr-reg =3D <010 2>;
+		};
+
+		MAL0: mcmal {
+			compatible =3D "ibm,mcmal-460sx", "ibm,mcmal2";
+			dcr-reg =3D <180 62>;
+			num-tx-chans =3D <4>;
+			num-rx-chans =3D <20>;
+			#address-cells =3D <1>;
+			#size-cells =3D <1>;
+			/*reg =3D <4 00040000 10000>;
+			ranges =3D <0 4 00040000 10000>;*/  /*OCM mapped

to 0xa0000000 */

You have reg and ranges commented out?  Not sure why those are here
anyway,
but you should remove them if they aren't needed.

+			interrupt-parent =3D <&UIC1>;
+			interrupts =3D <	/*TXEOB*/ 6 4
+					/*RXEOB*/ 7 4
+					/*SERR*/  1 4
+					/*TXDE*/  2 4
+					/*RXDE*/  3 4
+					/*COAL TX0*/ 18 2
+					/*COAL TX1*/ 19 2
+					/*COAL TX2*/ 1a 2
+					/*COAL TX3*/ 1b 2
+					/*COAL RX0*/ 1c 2
+					/*COAL RX1*/ 1d 2
+					/*COAL RX2*/ 1e 2
+					/*COAL RX3*/ 1f 2>;
+		};
+
+
+		POB0: opb {
+			compatible =3D "ibm,opb-460sx", "ibm,opb";
+			#address-cells =3D <1>;
+			#size-cells =3D <1>;
+			ranges =3D <b0000000 4 b0000000 50000000>;
+			clock-frequency =3D <0>; /* Filled in by U-Boot */
+
+			EBC0: ebc {
+				compatible =3D "ibm,ebc-460sx", "ibm,ebc";
+				dcr-reg =3D <012 2>;
+				#address-cells =3D <2>;
+				#size-cells =3D <1>;
+				clock-frequency =3D <0>; /* Filled in by

U-Boot */

+				/* ranges property is supplied by U-Boot

*/

+				interrupts =3D <6 4>;
+				interrupt-parent =3D <&UIC1>;
+
+				nor_flash@0,0 {
+					compatible =3D "amd,s29gl512n",

"cfi-flash";

+					bank-width =3D <2>;
+					reg =3D <0 000000 4000000>;
+					#address-cells =3D <1>;
+					#size-cells =3D <1>;
+					partition@0 {
+						label =3D "kernel";
+						reg =3D <0 1e0000>;
+					};
+					partition@1e0000 {
+						label =3D "dtb";
+						reg =3D <1e0000 20000>;
+					};
+					partition@200000 {
+						label =3D "ramdisk";
+						reg =3D <200000 1400000>;
+					};
+					partition@1600000 {
+						label =3D "jffs2";
+						reg =3D <1600000 400000>;
+					};
+					partition@1a00000 {
+						label =3D "user";
+						reg =3D <1a00000 2560000>;
+					};
+					partition@3f60000 {
+						label =3D "env";
+						reg =3D <3f60000 40000>;
+					};
+					partition@3fa0000 {
+						label =3D "u-boot";
+						reg =3D <3fa0000 60000>;
+					};
+				};
+			};
+
+			UART0: serial@ef600200 {
+				device_type =3D "serial";
+				compatible =3D "ns16550";
+				reg =3D <ef600200 8>;
+				virtual-reg =3D <ef600200>;
+				clock-frequency =3D <0>; /* Filled in by

U-Boot */

+				current-speed =3D <0>; /* Filled in by

U-Boot */

+				interrupt-parent =3D <&UIC0>;
+				interrupts =3D <0 4>;
+			};
+
+			RGMII0: emac-rgmii@ef600900 {
+				compatible =3D "ibm,rgmii-460sx",

"ibm,rgmii";

+				reg =3D <ef600900 8>;
+			};
+
+			EMAC0: ethernet@ef600a00 {
+				device_type =3D "network";
+				compatible =3D "ibm,emac-460sx",

"ibm,emac4";

+				interrupt-parent =3D <&EMAC0>;
+				interrupts =3D <0 1>;
+				#interrupt-cells =3D <1>;
+				#address-cells =3D <0>;
+				#size-cells =3D <0>;
+				interrupt-map =3D </*Status*/ 0 &UIC0 13 4
+						 /*Wake*/   1 &UIC2 1D

4>;

+				reg =3D <ef600a00 70>;
+				local-mac-address =3D [000000000000]; /*

Filled in by U-Boot */

I think you need to regenerate this defconfig against
a more current kernel.

Don't you also need __fixup_440A_mcheck here?

spec);

extern void __setup_cpu_604(unsigned long offset, struct cpu_spec*

spec);

extern void __setup_cpu_750(unsigned long offset, struct cpu_spec*

spec);

=3D
{

board.

+config REDWOOD
+	bool "Redwood"
+	depends on 44x
+	default n
+	select PPC44x_SIMPLE

If you are selecting PPC44x_SIMPLE, why do you create
your own redwood.c file?

+	select 460SX
+	select PCI
+	select PPC4xx_PCI_EXPRESS
+	help
+	  This option enables support for the AMCC PPC460SX validation

board.

above

above

config XILINX_VIRTEX_5_FXT
	bool
-	select XILINX_VIRTEX
-
+	select XILINX_VIRTEX
\ No newline at end of file

Erm, this is an unnecessary patch hunk.

Judging from the redwood.c file you create below, this is
entirely incorrect.

modify=20

it
+ * under  the terms of  the GNU General  Public License as published

by=20

the
+ * Free Software Foundation;  either version 2 of the  License, or (at

your
+ * option) any later version.
+ */

Word-wrapped.

+#include <linux/init.h>
+#include <linux/of_platform.h>
+
+#include <asm/machdep.h>
+#include <asm/prom.h>
+#include <asm/udbg.h>
+#include <asm/time.h>
+#include <asm/uic.h>
+#include <asm/pci-bridge.h>
+#include <asm/ppc4xx.h>
+#include <asm/dcr.h>
+#include <asm/dcr-regs.h>
+#include <asm/io.h>
+
+#define DCRN_EBC0_CONFIG_ADDR    0x012
+#define DCRN_EBC0_CONFIG_DATA    0x013
+
+static __initdata struct of_device_id redwood_of_bus[] =3D {
+	{ .compatible =3D "ibm,plb4", },
+	{ .compatible =3D "ibm,opb", },
+	{ .compatible =3D "ibm,ebc", },
+	{},
+};
+
+static int __init redwood_device_probe(void)
+{
+	of_platform_bus_probe(NULL, redwood_of_bus, NULL);
+
+	return 0;
+}
+machine_device_initcall(redwood, redwood_device_probe);
+
+static int __init redwood_probe(void)
+{
+	unsigned long root =3D of_get_flat_dt_root();
+
+	if (!of_flat_dt_is_compatible(root, "amcc,redwood"))
+		return 0;
+	ppc_pci_flags =3D PPC_PCI_REASSIGN_ALL_RSRC;
+	return 1;
+}
+
+static void __init redwood_setup_arch(void)
+{
+	struct device_node *np;
+	unsigned int *cpld_ptr =3D NULL;
+	unsigned int ebc_b3cr =3D 0;
+	unsigned long long ebc_cpld_addr =3D 0;
+
+	mtdcr(DCRN_EBC0_CONFIG_ADDR, 0x3);

What is 0x3?  No hard-coded magic hex values please.  Same comment
elsewhere.

+	ebc_b3cr =3D mfdcr(DCRN_EBC0_CONFIG_DATA);
+	/* cpld address retrieved from EBC_CR */
+	ebc_cpld_addr =3D 0x400000000ULL | (ebc_b3cr & 0xFFF00000);
+	cpld_ptr =3D ioremap(ebc_cpld_addr, 0xC);

Why aren't you using the device tree here?  Getting the cpld address
and resources should be easy enough if the cpld is enumerated in the
device tree...

+	if (!cpld_ptr) {
+		printk(KERN_ERR "Err: can't map CPLD registers!\n");
+		return;
+	}
+	/* Check EMAC bridge setting */
+	np =3D of_find_compatible_node(NULL, NULL, "ibm,emac-460sx");

of_find_compatible_node creates a reference on the node pointer.  You
need to use of_node_put on it when you are done.

+	if (np) {
+		const char *phymode;
+		unsigned int bcr;
+		phymode =3D of_get_property(np, "phy-mode", NULL);
+		if (!phymode) {
+			printk(KERN_ERR "Err: can't access node property

\

+					phy-mode\n defaulting to

rgmii");

+		}
+		bcr =3D in_be32(cpld_ptr + 2);
+		if (strcasecmp(phymode, "gmii") =3D=3D 0)
+			out_be32(cpld_ptr + 2, bcr & 0xFFEFFFFF);
+		else
+			out_be32(cpld_ptr + 2, bcr | 0x00100000);
+	}

A comment about what this function is trying to do would be nice.  I can
guess that it's setting the phy in a certain mode with some magical hex
values, but I have no idea why.

+}
+
+define_machine(redwood) {
+	.name 				=3D "redwood",
+	.probe 				=3D redwood_probe,
+	.setup_arch			=3D redwood_setup_arch,
+	.progress 			=3D udbg_progress,
+	.init_IRQ 			=3D uic_init_tree,
+	.get_irq 			=3D uic_get_irq,
+	.restart			=3D ppc4xx_reset_system,
+	.calibrate_decr			=3D generic_calibrate_decr,
+};
--=20
1.5.5
--------------------------------------------------------

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,

is=20

for the sole use of the intended recipient(s) and contains information=20
that is confidential and proprietary to Applied Micro Circuits

Corporation=20

or its subsidiaries. It is to be used solely for the purpose of

furthering=20

the parties' business relationship. All unauthorized review, use,=20
disclosure or distribution is prohibited. If you are not the intended=20
recipient, please contact the sender by reply e-mail and destroy all=20
copies of the original message.

Get rid of this message entirely.  It has no place on a public mailing
list,
and certainly not for a patch submission.

josh