[PATCH 1/7] powerpc: Add mpc8360epb platform support

[PATCH 1/7] powerpc: Add mpc8360epb platform support

[Li Yang-r58472]

The patch adds mpc8360e MDS Processor Board support.

Signed-off-by: Li Yang <leoli@freescale.com>
Signed-off-by: Yin Olivia <hong-hua.yin@freescale.com>
Signed-off-by: Kim Phillips <kim.phillips@freescale.com>

---
 arch/powerpc/platforms/83xx/Kconfig       |   13 ++
 arch/powerpc/platforms/83xx/Makefile      |    1 
 arch/powerpc/platforms/83xx/mpc8360e_pb.c |  213 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/83xx/mpc8360e_pb.h |   31 ++++
 4 files changed, 258 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/platforms/83xx/Kconfig
index 7675e67..04c4508 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -16,6 +16,13 @@ config MPC834x_SYS
 	  3 PCI slots.  The PIBs PCI initialization is the bootloader's
 	  responsiblilty.
 
+config MPC8360E_PB
+	bool "Freescale MPC8360E PB"
+	select DEFAULT_UIMAGE
+	select QUICC_ENGINE
+	help
+	  This option enables support for the MPC836x EMDS Processor Board.
+
 endchoice
 
 config MPC834x
@@ -24,4 +31,10 @@ config MPC834x
 	select PPC_INDIRECT_PCI
 	default y if MPC834x_SYS
 
+config MPC836x
+	bool
+	select PPC_UDBG_16550
+	select PPC_INDIRECT_PCI
+	default y if MPC8360E_PB
+
 endmenu
diff --git a/arch/powerpc/platforms/83xx/Makefile b/arch/powerpc/platforms/83xx/Makefile
index 5c72367..0c9ea5c 100644
--- a/arch/powerpc/platforms/83xx/Makefile
+++ b/arch/powerpc/platforms/83xx/Makefile
@@ -4,3 +4,4 @@ #
 obj-y				:= misc.o
 obj-$(CONFIG_PCI)		+= pci.o
 obj-$(CONFIG_MPC834x_SYS)	+= mpc834x_sys.o
+obj-$(CONFIG_MPC8360E_PB)	+= mpc8360e_pb.o
diff --git a/arch/powerpc/platforms/83xx/mpc8360e_pb.c b/arch/powerpc/platforms/83xx/mpc8360e_pb.c
new file mode 100644
index 0000000..b4ddb0a
--- /dev/null
+++ b/arch/powerpc/platforms/83xx/mpc8360e_pb.c
@@ -0,0 +1,213 @@
+/*
+ * Copyright (C) Freescale Semicondutor, Inc. 2006. All rights reserved.
+ *
+ * Author: Li Yang <LeoLi@freescale.com>
+ *	   Yin Olivia <Hong-hua.Yin@freescale.com>
+ *
+ * Description:
+ * MPC8360E MDS PB board specific routines. 
+ *
+ * Changelog: 
+ * Jun 21, 2006	Initial version
+ *
+ * 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.
+ */
+
+#include <linux/config.h>
+#include <linux/stddef.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/reboot.h>
+#include <linux/pci.h>
+#include <linux/kdev_t.h>
+#include <linux/major.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/seq_file.h>
+#include <linux/root_dev.h>
+#include <linux/initrd.h>
+
+#include <asm/system.h>
+#include <asm/atomic.h>
+#include <asm/time.h>
+#include <asm/io.h>
+#include <asm/machdep.h>
+#include <asm/ipic.h>
+#include <asm/bootinfo.h>
+#include <asm/irq.h>
+#include <asm/prom.h>
+#include <asm/udbg.h>
+#include <sysdev/fsl_soc.h>
+#ifdef CONFIG_QUICC_ENGINE
+#include <asm/immap_qe.h>
+#include <asm/qe_ic.h>
+#endif				/* CONFIG_QUICC_ENGINE */
+#include "mpc83xx.h"
+#include "mpc8360e_pb.h"
+
+#undef DEBUG
+
+#ifdef DEBUG
+#define DBG(fmt...) udbg_printf(fmt)
+#else
+#define DBG(fmt...)
+#endif
+
+
+#ifndef CONFIG_PCI
+unsigned long isa_io_base = 0;
+unsigned long isa_mem_base = 0;
+#endif
+
+#ifdef CONFIG_QUICC_ENGINE
+extern void qe_reset(void);
+extern int par_io_of_config(struct device_node *np);
+#endif	/* CONFIG_QUICC_ENGINE */
+
+/* ************************************************************************
+ *
+ * Setup the architecture
+ *
+ */
+static void __init mpc8360_sys_setup_arch(void)
+{
+	struct device_node *np;
+	
+#ifdef CONFIG_QUICC_ENGINE
+	u8 *bcsr_regs;
+#endif
+
+	if (ppc_md.progress)
+		ppc_md.progress("mpc8360_sys_setup_arch()", 0);
+
+	np = of_find_node_by_type(NULL, "cpu");
+	if (np != 0) {
+		unsigned int *fp =
+		    (int *)get_property(np, "clock-frequency", NULL);
+		if (fp != 0)
+			loops_per_jiffy = *fp / HZ;
+		else
+			loops_per_jiffy = 50000000 / HZ;
+		of_node_put(np);
+	}
+#ifdef CONFIG_PCI
+	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
+		add_bridge(np);
+
+	ppc_md.pci_swizzle = common_swizzle;
+	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
+#endif
+
+#ifdef CONFIG_QUICC_ENGINE
+	qe_reset();
+
+	for (np = NULL; (np = of_find_node_by_name(np, "ucc")) != NULL;)
+		par_io_of_config(np);
+	
+	/* Reset the Ethernet PHY */
+	bcsr_regs = (u8 *) ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
+	bcsr_regs[9] &= ~0x20;
+	udelay(1000);
+	bcsr_regs[9] |= 0x20;
+	iounmap(bcsr_regs);
+
+#endif				/* CONFIG_QUICC_ENGINE */
+
+#ifdef CONFIG_BLK_DEV_INITRD
+	if (initrd_start)
+		ROOT_DEV = Root_RAM0;
+	else
+#endif
+#ifdef  CONFIG_ROOT_NFS
+		ROOT_DEV = Root_NFS;
+#else
+		ROOT_DEV = Root_HDA1;
+#endif
+}
+
+void __init mpc8360_sys_init_IRQ(void)
+{
+	u8 senses[8] = {
+		0,		/* EXT 0 */
+		IRQ_SENSE_LEVEL,	/* EXT 1 */
+		IRQ_SENSE_LEVEL,	/* EXT 2 */
+		0,		/* EXT 3 */
+#ifdef CONFIG_PCI
+		IRQ_SENSE_LEVEL,	/* EXT 4 */
+		IRQ_SENSE_LEVEL,	/* EXT 5 */
+		IRQ_SENSE_LEVEL,	/* EXT 6 */
+		IRQ_SENSE_LEVEL,	/* EXT 7 */
+#else
+		0,		/* EXT 4 */
+		0,		/* EXT 5 */
+		0,		/* EXT 6 */
+		0,		/* EXT 7 */
+#endif
+	};
+
+	ipic_init(get_immrbase() + 0x00700, 0, 0, senses, 8);
+
+	/* Initialize the default interrupt mapping priorities,
+	 * in case the boot rom changed something on us.
+	 */
+	ipic_set_default_priority();
+
+#ifdef CONFIG_QUICC_ENGINE
+	qe_ic_init(get_qe_base() + 0x00000080,
+		   (QE_IC_LOW_SIGNAL | QE_IC_HIGH_SIGNAL), QE_IRQ_OFFSET);
+#endif				/* CONFIG_QUICC_ENGINE */
+}
+
+#if defined(CONFIG_I2C_MPC) && defined(CONFIG_SENSORS_DS1374)
+extern ulong ds1374_get_rtc_time(void);
+extern int ds1374_set_rtc_time(ulong);
+
+static int __init mpc8360_rtc_hookup(void)
+{
+	struct timespec tv;
+
+	ppc_md.get_rtc_time = ds1374_get_rtc_time;
+	ppc_md.set_rtc_time = ds1374_set_rtc_time;
+
+	tv.tv_nsec = 0;
+	tv.tv_sec = (ppc_md.get_rtc_time) ();
+	do_settimeofday(&tv);
+
+	return 0;
+}
+
+late_initcall(mpc8360_rtc_hookup);
+#endif
+
+/*
+ * Called very early, MMU is off, device-tree isn't unflattened
+ */
+static int __init mpc8360_sys_probe(void)
+{
+	char *model = of_get_flat_dt_prop(of_get_flat_dt_root(),
+					  "model", NULL);
+	if (model == NULL)
+		return 0;
+	if (strcmp(model, "MPC8360EPB"))
+		return 0;
+
+	DBG("MPC8360EMDS-PB found\n");
+
+	return 1;
+}
+
+define_machine(mpc8360_sys) {
+	.name 		= "MPC8360E PB",
+	.probe 		= mpc8360_sys_probe,
+	.setup_arch 	= mpc8360_sys_setup_arch,
+	.init_IRQ 	= mpc8360_sys_init_IRQ,
+	.get_irq 	= ipic_get_irq,
+	.restart 	= mpc83xx_restart,
+	.time_init 	= mpc83xx_time_init,
+	.calibrate_decr	= generic_calibrate_decr,
+	.progress 	= udbg_progress,
+};
diff --git a/arch/powerpc/platforms/83xx/mpc8360e_pb.h b/arch/powerpc/platforms/83xx/mpc8360e_pb.h
new file mode 100644
index 0000000..4243f4a
--- /dev/null
+++ b/arch/powerpc/platforms/83xx/mpc8360e_pb.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) Freescale Semicondutor, Inc. 2006. All rights reserved.
+ *
+ * Author: Li Yang <LeoLi@freescale.com>
+ *	   Yin Olivia <Hong-hua.Yin@freescale.com>
+ *
+ * Description:
+ * MPC8360E MDS PB board specific header. 
+ *
+ * Changelog: 
+ * Jun 21, 2006	Initial version
+ *
+ * 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.
+ *
+ */
+
+#ifndef __MACH_MPC83XX_SYS_H__
+#define __MACH_MPC83XX_SYS_H__
+
+#define BCSR_PHYS_ADDR		((uint)0xf8000000)
+#define BCSR_SIZE		((uint)(32 * 1024))
+
+#define PIRQA	MPC83xx_IRQ_EXT4
+#define PIRQB	MPC83xx_IRQ_EXT5
+#define PIRQC	MPC83xx_IRQ_EXT6
+#define PIRQD	MPC83xx_IRQ_EXT7
+
+#endif				/* __MACH_MPC83XX_SYS_H__ */
 

Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Vitaly Bordug]

On Wed, 28 Jun 2006 22:23:03 +0800
Li Yang-r58472 <LeoLi@freescale.com> wrote:

> The patch adds mpc8360e MDS Processor Board support.
Far too short comment I guess.. There should be some information at least, what u-boot modifications are required, what family being introduced, etc.

> 
> Signed-off-by: Li Yang <leoli@freescale.com>
> Signed-off-by: Yin Olivia <hong-hua.yin@freescale.com>
> Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
> 
> ---
>  arch/powerpc/platforms/83xx/Kconfig       |   13 ++
>  arch/powerpc/platforms/83xx/Makefile      |    1 
>  arch/powerpc/platforms/83xx/mpc8360e_pb.c |  213 +++++++++++++++++++++++++++++
>  arch/powerpc/platforms/83xx/mpc8360e_pb.h |   31 ++++
>  4 files changed, 258 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/platforms/83xx/Kconfig
> index 7675e67..04c4508 100644
> --- a/arch/powerpc/platforms/83xx/Kconfig
> +++ b/arch/powerpc/platforms/83xx/Kconfig
> @@ -16,6 +16,13 @@ config MPC834x_SYS
>  	  3 PCI slots.  The PIBs PCI initialization is the bootloader's
>  	  responsiblilty.
>  
> +config MPC8360E_PB
> +	bool "Freescale MPC8360E PB"
> +	select DEFAULT_UIMAGE
> +	select QUICC_ENGINE
> +	help
> +	  This option enables support for the MPC836x EMDS Processor Board.
> +
>  endchoice

I don't think this is really required option. I guess 836x + QUICC_ENGINE should be enough (with a proviso that 8360 won't boot without qe.

>  
>  config MPC834x
> @@ -24,4 +31,10 @@ config MPC834x
>  	select PPC_INDIRECT_PCI
>  	default y if MPC834x_SYS
>  
> +config MPC836x
> +	bool
> +	select PPC_UDBG_16550

debug option made default?
> +	select PPC_INDIRECT_PCI
> +	default y if MPC8360E_PB
> +
>  endmenu
> diff --git a/arch/powerpc/platforms/83xx/Makefile b/arch/powerpc/platforms/83xx/Makefile
> index 5c72367..0c9ea5c 100644
> --- a/arch/powerpc/platforms/83xx/Makefile
> +++ b/arch/powerpc/platforms/83xx/Makefile
> @@ -4,3 +4,4 @@ #
>  obj-y				:= misc.o
>  obj-$(CONFIG_PCI)		+= pci.o
>  obj-$(CONFIG_MPC834x_SYS)	+= mpc834x_sys.o
> +obj-$(CONFIG_MPC8360E_PB)	+= mpc8360e_pb.o
> diff --git a/arch/powerpc/platforms/83xx/mpc8360e_pb.c b/arch/powerpc/platforms/83xx/mpc8360e_pb.c
> new file mode 100644
> index 0000000..b4ddb0a
> --- /dev/null
> +++ b/arch/powerpc/platforms/83xx/mpc8360e_pb.c
> @@ -0,0 +1,213 @@
> +/*
> + * Copyright (C) Freescale Semicondutor, Inc. 2006. All rights reserved.
> + *
> + * Author: Li Yang <LeoLi@freescale.com>
> + *	   Yin Olivia <Hong-hua.Yin@freescale.com>
> + *
> + * Description:
> + * MPC8360E MDS PB board specific routines. 
> + *
> + * Changelog: 
> + * Jun 21, 2006	Initial version
> + *
No changelog entries for new files please... git tracks it good enough.

> + * 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.
> + */
> +
> +#include <linux/config.h>
> +#include <linux/stddef.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/reboot.h>
> +#include <linux/pci.h>
> +#include <linux/kdev_t.h>
> +#include <linux/major.h>
> +#include <linux/console.h>
> +#include <linux/delay.h>
> +#include <linux/seq_file.h>
> +#include <linux/root_dev.h>
> +#include <linux/initrd.h>
> +
> +#include <asm/system.h>
> +#include <asm/atomic.h>
> +#include <asm/time.h>
> +#include <asm/io.h>
> +#include <asm/machdep.h>
> +#include <asm/ipic.h>
> +#include <asm/bootinfo.h>
> +#include <asm/irq.h>
> +#include <asm/prom.h>
> +#include <asm/udbg.h>
> +#include <sysdev/fsl_soc.h>
> +#ifdef CONFIG_QUICC_ENGINE
> +#include <asm/immap_qe.h>
> +#include <asm/qe_ic.h>
> +#endif				/* CONFIG_QUICC_ENGINE */
> +#include "mpc83xx.h"
> +#include "mpc8360e_pb.h"
> +
> +#undef DEBUG
> +

hmmm? Does it relate nicely with below ?
> +#ifdef DEBUG
> +#define DBG(fmt...) udbg_printf(fmt)
> +#else
> +#define DBG(fmt...)
> +#endif
> +
> +
> +#ifndef CONFIG_PCI
> +unsigned long isa_io_base = 0;
> +unsigned long isa_mem_base = 0;
> +#endif
> +
> +#ifdef CONFIG_QUICC_ENGINE
> +extern void qe_reset(void);
> +extern int par_io_of_config(struct device_node *np);
> +#endif	/* CONFIG_QUICC_ENGINE */

I bet this should go to the .h file...
> +
> +/* ************************************************************************
> + *
> + * Setup the architecture
> + *
> + */
> +static void __init mpc8360_sys_setup_arch(void)
> +{
> +	struct device_node *np;
> +	
> +#ifdef CONFIG_QUICC_ENGINE
> +	u8 *bcsr_regs;
> +#endif
> +
> +	if (ppc_md.progress)
> +		ppc_md.progress("mpc8360_sys_setup_arch()", 0);
> +
> +	np = of_find_node_by_type(NULL, "cpu");
> +	if (np != 0) {
> +		unsigned int *fp =
> +		    (int *)get_property(np, "clock-frequency", NULL);
> +		if (fp != 0)
> +			loops_per_jiffy = *fp / HZ;
> +		else
> +			loops_per_jiffy = 50000000 / HZ;
> +		of_node_put(np);
> +	}
> +#ifdef CONFIG_PCI
> +	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
> +		add_bridge(np);
> +
> +	ppc_md.pci_swizzle = common_swizzle;
> +	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
> +#endif
> +
> +#ifdef CONFIG_QUICC_ENGINE
> +	qe_reset();
> +
> +	for (np = NULL; (np = of_find_node_by_name(np, "ucc")) != NULL;)
> +		par_io_of_config(np);
> +	
> +	/* Reset the Ethernet PHY */
> +	bcsr_regs = (u8 *) ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
> +	bcsr_regs[9] &= ~0x20;
> +	udelay(1000);
> +	bcsr_regs[9] |= 0x20;
> +	iounmap(bcsr_regs);
> +
And if we have a design, which do not contain real ethernet UCC usage? Or UCC geth is disabled somehow explicitly? Stuff like that normally goes to the callback that is going to be triggered upon Etherbet init.


> +#endif				/* CONFIG_QUICC_ENGINE */
> +
> +#ifdef CONFIG_BLK_DEV_INITRD
> +	if (initrd_start)
> +		ROOT_DEV = Root_RAM0;
> +	else
> +#endif
> +#ifdef  CONFIG_ROOT_NFS
> +		ROOT_DEV = Root_NFS;
> +#else
> +		ROOT_DEV = Root_HDA1;
> +#endif
> +}
> +
> +void __init mpc8360_sys_init_IRQ(void)
> +{
> +	u8 senses[8] = {
> +		0,		/* EXT 0 */
> +		IRQ_SENSE_LEVEL,	/* EXT 1 */
> +		IRQ_SENSE_LEVEL,	/* EXT 2 */
> +		0,		/* EXT 3 */
> +#ifdef CONFIG_PCI
> +		IRQ_SENSE_LEVEL,	/* EXT 4 */
> +		IRQ_SENSE_LEVEL,	/* EXT 5 */
> +		IRQ_SENSE_LEVEL,	/* EXT 6 */
> +		IRQ_SENSE_LEVEL,	/* EXT 7 */
> +#else
> +		0,		/* EXT 4 */
> +		0,		/* EXT 5 */
> +		0,		/* EXT 6 */
> +		0,		/* EXT 7 */
> +#endif
> +	};
> +
> +	ipic_init(get_immrbase() + 0x00700, 0, 0, senses, 8);
> +
> +	/* Initialize the default interrupt mapping priorities,
> +	 * in case the boot rom changed something on us.
> +	 */
> +	ipic_set_default_priority();
> +
> +#ifdef CONFIG_QUICC_ENGINE
> +	qe_ic_init(get_qe_base() + 0x00000080,
> +		   (QE_IC_LOW_SIGNAL | QE_IC_HIGH_SIGNAL), QE_IRQ_OFFSET);
> +#endif				/* CONFIG_QUICC_ENGINE */
> +}
> +
> +#if defined(CONFIG_I2C_MPC) && defined(CONFIG_SENSORS_DS1374)
> +extern ulong ds1374_get_rtc_time(void);
> +extern int ds1374_set_rtc_time(ulong);
> +
> +static int __init mpc8360_rtc_hookup(void)
> +{
> +	struct timespec tv;
> +
> +	ppc_md.get_rtc_time = ds1374_get_rtc_time;
> +	ppc_md.set_rtc_time = ds1374_set_rtc_time;
> +
> +	tv.tv_nsec = 0;
> +	tv.tv_sec = (ppc_md.get_rtc_time) ();
> +	do_settimeofday(&tv);
> +
> +	return 0;
> +}
> +
> +late_initcall(mpc8360_rtc_hookup);
> +#endif
> +
> +/*
> + * Called very early, MMU is off, device-tree isn't unflattened
> + */
> +static int __init mpc8360_sys_probe(void)
> +{
> +	char *model = of_get_flat_dt_prop(of_get_flat_dt_root(),
> +					  "model", NULL);
> +	if (model == NULL)
> +		return 0;
> +	if (strcmp(model, "MPC8360EPB"))
> +		return 0;
> +
> +	DBG("MPC8360EMDS-PB found\n");
> +
> +	return 1;
> +}
> +
> +define_machine(mpc8360_sys) {
> +	.name 		= "MPC8360E PB",
> +	.probe 		= mpc8360_sys_probe,
> +	.setup_arch 	= mpc8360_sys_setup_arch,
> +	.init_IRQ 	= mpc8360_sys_init_IRQ,
> +	.get_irq 	= ipic_get_irq,
> +	.restart 	= mpc83xx_restart,
> +	.time_init 	= mpc83xx_time_init,
> +	.calibrate_decr	= generic_calibrate_decr,
> +	.progress 	= udbg_progress,
> +};
> diff --git a/arch/powerpc/platforms/83xx/mpc8360e_pb.h b/arch/powerpc/platforms/83xx/mpc8360e_pb.h
> new file mode 100644
> index 0000000..4243f4a
> --- /dev/null
> +++ b/arch/powerpc/platforms/83xx/mpc8360e_pb.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (C) Freescale Semicondutor, Inc. 2006. All rights reserved.
> + *
> + * Author: Li Yang <LeoLi@freescale.com>
> + *	   Yin Olivia <Hong-hua.Yin@freescale.com>
> + *
> + * Description:
> + * MPC8360E MDS PB board specific header. 
> + *
> + * Changelog: 
> + * Jun 21, 2006	Initial version
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef __MACH_MPC83XX_SYS_H__
> +#define __MACH_MPC83XX_SYS_H__
> +
> +#define BCSR_PHYS_ADDR		((uint)0xf8000000)
> +#define BCSR_SIZE		((uint)(32 * 1024))
> +
> +#define PIRQA	MPC83xx_IRQ_EXT4
> +#define PIRQB	MPC83xx_IRQ_EXT5
> +#define PIRQC	MPC83xx_IRQ_EXT6
> +#define PIRQD	MPC83xx_IRQ_EXT7
> +

Hrm, isn't PCI irq stuff encoded to the dts? Upper pci-related defines seem redundant...

> +#endif				/* __MACH_MPC83XX_SYS_H__ */
>  
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 
> 


-- 
Sincerely, 
Vitaly

RE: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Li Yang-r58472]

> -----Original Message-----
> From: Vitaly Bordug [mailto:vbordug@ru.mvista.com]
> Sent: Thursday, June 29, 2006 12:59 AM
> To: Li Yang-r58472
> Cc: 'Paul Mackerras'; linuxppc-dev@ozlabs.org; Phillips Kim-R1AAHA; Chu
> hanjin-r52514; Yin Olivia-r63875; 'linux-kernel@vger.kernel.org'
> Subject: Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support
> 
> On Wed, 28 Jun 2006 22:23:03 +0800
> Li Yang-r58472 <LeoLi@freescale.com> wrote:
> 
> > The patch adds mpc8360e MDS Processor Board support.
> Far too short comment I guess.. There should be some information at least, what
> u-boot modifications are required, what family being introduced, etc.

The new revision goes like this:
The patch adds mpc8360e MDS Processor Board support.  MPC8360E is a new processor in the PowerQUICC II pro family, and it's the first one to use the new generation of communication coprocessor called QUICC ENGINE (QE in abbreviation).
The patch needs a flat device tree to work.  It means either u-boot provides a flat device tree blob or somehow a kernel shim passes the tree.  The final mechanism of providing FDT is far beyond this patch.
> 
> >
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > Signed-off-by: Yin Olivia <hong-hua.yin@freescale.com>
> > Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
> >
> > ---
> >  arch/powerpc/platforms/83xx/Kconfig       |   13 ++
> >  arch/powerpc/platforms/83xx/Makefile      |    1
> >  arch/powerpc/platforms/83xx/mpc8360e_pb.c |  213
> +++++++++++++++++++++++++++++
> >  arch/powerpc/platforms/83xx/mpc8360e_pb.h |   31 ++++
> >  4 files changed, 258 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/83xx/Kconfig
> b/arch/powerpc/platforms/83xx/Kconfig
> > index 7675e67..04c4508 100644
> > --- a/arch/powerpc/platforms/83xx/Kconfig
> > +++ b/arch/powerpc/platforms/83xx/Kconfig
> > @@ -16,6 +16,13 @@ config MPC834x_SYS
> >  	  3 PCI slots.  The PIBs PCI initialization is the bootloader's
> >  	  responsiblilty.
> >
> > +config MPC8360E_PB
> > +	bool "Freescale MPC8360E PB"
> > +	select DEFAULT_UIMAGE
> > +	select QUICC_ENGINE
> > +	help
> > +	  This option enables support for the MPC836x EMDS Processor Board.
> > +
> >  endchoice
> 
> I don't think this is really required option. I guess 836x + QUICC_ENGINE should
> be enough (with a proviso that 8360 won't boot without qe.

We select a board and the board implies cpu family and soc feature.  That will be better for users rather than expecting them to know the very detail.
> 
> >
> >  config MPC834x
> > @@ -24,4 +31,10 @@ config MPC834x
> >  	select PPC_INDIRECT_PCI
> >  	default y if MPC834x_SYS
> >
> > +config MPC836x
> > +	bool
> > +	select PPC_UDBG_16550
> 
> debug option made default?

good catch.
> > +	select PPC_INDIRECT_PCI
> > +	default y if MPC8360E_PB
> > +
> >  endmenu
> > diff --git a/arch/powerpc/platforms/83xx/Makefile
> b/arch/powerpc/platforms/83xx/Makefile
> > index 5c72367..0c9ea5c 100644
> > --- a/arch/powerpc/platforms/83xx/Makefile
> > +++ b/arch/powerpc/platforms/83xx/Makefile
> > @@ -4,3 +4,4 @@ #
> >  obj-y				:= misc.o
> >  obj-$(CONFIG_PCI)		+= pci.o
> >  obj-$(CONFIG_MPC834x_SYS)	+= mpc834x_sys.o
> > +obj-$(CONFIG_MPC8360E_PB)	+= mpc8360e_pb.o
> > diff --git a/arch/powerpc/platforms/83xx/mpc8360e_pb.c
> b/arch/powerpc/platforms/83xx/mpc8360e_pb.c
> > new file mode 100644
> > index 0000000..b4ddb0a
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/83xx/mpc8360e_pb.c
> > @@ -0,0 +1,213 @@
> > +/*
> > + * Copyright (C) Freescale Semicondutor, Inc. 2006. All rights reserved.
> > + *
> > + * Author: Li Yang <LeoLi@freescale.com>
> > + *	   Yin Olivia <Hong-hua.Yin@freescale.com>
> > + *
> > + * Description:
> > + * MPC8360E MDS PB board specific routines.
> > + *
> > + * Changelog:
> > + * Jun 21, 2006	Initial version
> > + *
> No changelog entries for new files please... git tracks it good enough.

This is Freescale protocol.  If it is not welcomed, we will change it.
> 
> > + * 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.
> > + */
> > +
> > +#include <linux/config.h>
> > +#include <linux/stddef.h>
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/errno.h>
> > +#include <linux/reboot.h>
> > +#include <linux/pci.h>
> > +#include <linux/kdev_t.h>
> > +#include <linux/major.h>
> > +#include <linux/console.h>
> > +#include <linux/delay.h>
> > +#include <linux/seq_file.h>
> > +#include <linux/root_dev.h>
> > +#include <linux/initrd.h>
> > +
> > +#include <asm/system.h>
> > +#include <asm/atomic.h>
> > +#include <asm/time.h>
> > +#include <asm/io.h>
> > +#include <asm/machdep.h>
> > +#include <asm/ipic.h>
> > +#include <asm/bootinfo.h>
> > +#include <asm/irq.h>
> > +#include <asm/prom.h>
> > +#include <asm/udbg.h>
> > +#include <sysdev/fsl_soc.h>
> > +#ifdef CONFIG_QUICC_ENGINE
> > +#include <asm/immap_qe.h>
> > +#include <asm/qe_ic.h>
> > +#endif				/* CONFIG_QUICC_ENGINE */
> > +#include "mpc83xx.h"
> > +#include "mpc8360e_pb.h"
> > +
> > +#undef DEBUG
> > +
> 
> hmmm? Does it relate nicely with below ?
> > +#ifdef DEBUG
> > +#define DBG(fmt...) udbg_printf(fmt)
> > +#else
> > +#define DBG(fmt...)
> > +#endif
> > +
> > +
> > +#ifndef CONFIG_PCI
> > +unsigned long isa_io_base = 0;
> > +unsigned long isa_mem_base = 0;
> > +#endif
> > +
> > +#ifdef CONFIG_QUICC_ENGINE
> > +extern void qe_reset(void);
> > +extern int par_io_of_config(struct device_node *np);
> > +#endif	/* CONFIG_QUICC_ENGINE */
> 
> I bet this should go to the .h file...
> > +
> > +/*
> ************************************************************************
> > + *
> > + * Setup the architecture
> > + *
> > + */
> > +static void __init mpc8360_sys_setup_arch(void)
> > +{
> > +	struct device_node *np;
> > +
> > +#ifdef CONFIG_QUICC_ENGINE
> > +	u8 *bcsr_regs;
> > +#endif
> > +
> > +	if (ppc_md.progress)
> > +		ppc_md.progress("mpc8360_sys_setup_arch()", 0);
> > +
> > +	np = of_find_node_by_type(NULL, "cpu");
> > +	if (np != 0) {
> > +		unsigned int *fp =
> > +		    (int *)get_property(np, "clock-frequency", NULL);
> > +		if (fp != 0)
> > +			loops_per_jiffy = *fp / HZ;
> > +		else
> > +			loops_per_jiffy = 50000000 / HZ;
> > +		of_node_put(np);
> > +	}
> > +#ifdef CONFIG_PCI
> > +	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
> > +		add_bridge(np);
> > +
> > +	ppc_md.pci_swizzle = common_swizzle;
> > +	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
> > +#endif
> > +
> > +#ifdef CONFIG_QUICC_ENGINE
> > +	qe_reset();
> > +
> > +	for (np = NULL; (np = of_find_node_by_name(np, "ucc")) != NULL;)
> > +		par_io_of_config(np);
> > +
> > +	/* Reset the Ethernet PHY */
> > +	bcsr_regs = (u8 *) ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
> > +	bcsr_regs[9] &= ~0x20;
> > +	udelay(1000);
> > +	bcsr_regs[9] |= 0x20;
> > +	iounmap(bcsr_regs);
> > +
> And if we have a design, which do not contain real ethernet UCC usage? Or UCC
> geth is disabled somehow explicitly? Stuff like that normally goes to the
> callback that is going to be triggered upon Etherbet init.
I will move it.
> 
> 
> > +#endif				/* CONFIG_QUICC_ENGINE */
> > +
> > +#ifdef CONFIG_BLK_DEV_INITRD
> > +	if (initrd_start)
> > +		ROOT_DEV = Root_RAM0;
> > +	else
> > +#endif
> > +#ifdef  CONFIG_ROOT_NFS
> > +		ROOT_DEV = Root_NFS;
> > +#else
> > +		ROOT_DEV = Root_HDA1;
> > +#endif
> > +}
> > +
> > +void __init mpc8360_sys_init_IRQ(void)
> > +{
> > +	u8 senses[8] = {
> > +		0,		/* EXT 0 */
> > +		IRQ_SENSE_LEVEL,	/* EXT 1 */
> > +		IRQ_SENSE_LEVEL,	/* EXT 2 */
> > +		0,		/* EXT 3 */
> > +#ifdef CONFIG_PCI
> > +		IRQ_SENSE_LEVEL,	/* EXT 4 */
> > +		IRQ_SENSE_LEVEL,	/* EXT 5 */
> > +		IRQ_SENSE_LEVEL,	/* EXT 6 */
> > +		IRQ_SENSE_LEVEL,	/* EXT 7 */
> > +#else
> > +		0,		/* EXT 4 */
> > +		0,		/* EXT 5 */
> > +		0,		/* EXT 6 */
> > +		0,		/* EXT 7 */
> > +#endif
> > +	};
> > +
> > +	ipic_init(get_immrbase() + 0x00700, 0, 0, senses, 8);
> > +
> > +	/* Initialize the default interrupt mapping priorities,
> > +	 * in case the boot rom changed something on us.
> > +	 */
> > +	ipic_set_default_priority();
> > +
> > +#ifdef CONFIG_QUICC_ENGINE
> > +	qe_ic_init(get_qe_base() + 0x00000080,
> > +		   (QE_IC_LOW_SIGNAL | QE_IC_HIGH_SIGNAL), QE_IRQ_OFFSET);
> > +#endif				/* CONFIG_QUICC_ENGINE */
> > +}
> > +
> > +#if defined(CONFIG_I2C_MPC) && defined(CONFIG_SENSORS_DS1374)
> > +extern ulong ds1374_get_rtc_time(void);
> > +extern int ds1374_set_rtc_time(ulong);
> > +
> > +static int __init mpc8360_rtc_hookup(void)
> > +{
> > +	struct timespec tv;
> > +
> > +	ppc_md.get_rtc_time = ds1374_get_rtc_time;
> > +	ppc_md.set_rtc_time = ds1374_set_rtc_time;
> > +
> > +	tv.tv_nsec = 0;
> > +	tv.tv_sec = (ppc_md.get_rtc_time) ();
> > +	do_settimeofday(&tv);
> > +
> > +	return 0;
> > +}
> > +
> > +late_initcall(mpc8360_rtc_hookup);
> > +#endif
> > +
> > +/*
> > + * Called very early, MMU is off, device-tree isn't unflattened
> > + */
> > +static int __init mpc8360_sys_probe(void)
> > +{
> > +	char *model = of_get_flat_dt_prop(of_get_flat_dt_root(),
> > +					  "model", NULL);
> > +	if (model == NULL)
> > +		return 0;
> > +	if (strcmp(model, "MPC8360EPB"))
> > +		return 0;
> > +
> > +	DBG("MPC8360EMDS-PB found\n");
> > +
> > +	return 1;
> > +}
> > +
> > +define_machine(mpc8360_sys) {
> > +	.name 		= "MPC8360E PB",
> > +	.probe 		= mpc8360_sys_probe,
> > +	.setup_arch 	= mpc8360_sys_setup_arch,
> > +	.init_IRQ 	= mpc8360_sys_init_IRQ,
> > +	.get_irq 	= ipic_get_irq,
> > +	.restart 	= mpc83xx_restart,
> > +	.time_init 	= mpc83xx_time_init,
> > +	.calibrate_decr	= generic_calibrate_decr,
> > +	.progress 	= udbg_progress,
> > +};
> > diff --git a/arch/powerpc/platforms/83xx/mpc8360e_pb.h
> b/arch/powerpc/platforms/83xx/mpc8360e_pb.h
> > new file mode 100644
> > index 0000000..4243f4a
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/83xx/mpc8360e_pb.h
> > @@ -0,0 +1,31 @@
> > +/*
> > + * Copyright (C) Freescale Semicondutor, Inc. 2006. All rights reserved.
> > + *
> > + * Author: Li Yang <LeoLi@freescale.com>
> > + *	   Yin Olivia <Hong-hua.Yin@freescale.com>
> > + *
> > + * Description:
> > + * MPC8360E MDS PB board specific header.
> > + *
> > + * Changelog:
> > + * Jun 21, 2006	Initial version
> > + *
> > + * 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.
> > + *
> > + */
> > +
> > +#ifndef __MACH_MPC83XX_SYS_H__
> > +#define __MACH_MPC83XX_SYS_H__
> > +
> > +#define BCSR_PHYS_ADDR		((uint)0xf8000000)
> > +#define BCSR_SIZE		((uint)(32 * 1024))
> > +
> > +#define PIRQA	MPC83xx_IRQ_EXT4
> > +#define PIRQB	MPC83xx_IRQ_EXT5
> > +#define PIRQC	MPC83xx_IRQ_EXT6
> > +#define PIRQD	MPC83xx_IRQ_EXT7
> > +
> 
> Hrm, isn't PCI irq stuff encoded to the dts? Upper pci-related defines seem
> redundant...
I'll remove.
> 
> > +#endif				/* __MACH_MPC83XX_SYS_H__ */
> >
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@ozlabs.org
> > https://ozlabs.org/mailman/listinfo/linuxppc-dev
> >
> >
> 
> 
> --
> Sincerely,
> Vitaly

Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Andy Fleming]

On Jun 29, 2006, at 04:28, Li Yang-r58472 wrote:

>>> +config MPC8360E_PB
>>> +	bool "Freescale MPC8360E PB"
>>> +	select DEFAULT_UIMAGE
>>> +	select QUICC_ENGINE
>>> +	help
>>> +	  This option enables support for the MPC836x EMDS Processor  
>>> Board.
>>> +
>>>  endchoice
>>
>> I don't think this is really required option. I guess 836x +  
>> QUICC_ENGINE should
>> be enough (with a proviso that 8360 won't boot without qe.
>
> We select a board and the board implies cpu family and soc  
> feature.  That will be better for users rather than expecting them  
> to know the very detail.


Yeah, this seems fine to me.  In order to eliminate this option, we'd  
need to merge 8360 PB support with the existing board support for  
83xx.  A laudable goal, but not what the 83xx maintainer has  
currently requested.



>>> diff --git a/arch/powerpc/platforms/83xx/mpc8360e_pb.c
>> b/arch/powerpc/platforms/83xx/mpc8360e_pb.c
>>> new file mode 100644
>>> index 0000000..b4ddb0a
>>> --- /dev/null
>>> +++ b/arch/powerpc/platforms/83xx/mpc8360e_pb.c
>>> @@ -0,0 +1,213 @@
>>> +/*
>>> + * Copyright (C) Freescale Semicondutor, Inc. 2006. All rights  
>>> reserved.
>>> + *
>>> + * Author: Li Yang <LeoLi@freescale.com>
>>> + *	   Yin Olivia <Hong-hua.Yin@freescale.com>
>>> + *
>>> + * Description:
>>> + * MPC8360E MDS PB board specific routines.
>>> + *
>>> + * Changelog:
>>> + * Jun 21, 2006	Initial version
>>> + *
>> No changelog entries for new files please... git tracks it good  
>> enough.
>
> This is Freescale protocol.  If it is not welcomed, we will change it.


Yeah, this is one of the problems with the protocol.



>>> +#ifdef CONFIG_QUICC_ENGINE
>>> +	qe_reset();
>>> +
>>> +	for (np = NULL; (np = of_find_node_by_name(np, "ucc")) != NULL;)
>>> +		par_io_of_config(np);
>>> +
>>> +	/* Reset the Ethernet PHY */
>>> +	bcsr_regs = (u8 *) ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
>>> +	bcsr_regs[9] &= ~0x20;
>>> +	udelay(1000);
>>> +	bcsr_regs[9] |= 0x20;
>>> +	iounmap(bcsr_regs);
>>> +
>> And if we have a design, which do not contain real ethernet UCC  
>> usage? Or UCC
>> geth is disabled somehow explicitly? Stuff like that normally goes  
>> to the
>> callback that is going to be triggered upon Etherbet init.
> I will move it.


Wait...no.  I don't understand Vitaly's objection.  If someone  
creates a board with an 8360 that doesn't use the UCC ethernet, they  
can create a separate board file.  This is the board-specific code,  
and it is perfectly acceptable for it to reset the PHY like this.   
What ethernet callback could be used?

Andy

Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Vitaly Bordug]

On Thu, 29 Jun 2006 13:03:23 -0500
Andy Fleming wrote:

[snip]
> >>> +	iounmap(bcsr_regs);
> >>> +
> >> And if we have a design, which do not contain real ethernet UCC  
> >> usage? Or UCC
> >> geth is disabled somehow explicitly? Stuff like that normally
> >> goes to the
> >> callback that is going to be triggered upon Etherbet init.
> > I will move it.
> 
> 
> Wait...no.  I don't understand Vitaly's objection.  If someone  
> creates a board with an 8360 that doesn't use the UCC ethernet, they  
> can create a separate board file.  This is the board-specific code,  
> and it is perfectly acceptable for it to reset the PHY like this.   
> What ethernet callback could be used?
> 

I am sort of against the unconditional trigger of the ethernet-specific stuff,
dependless if UCC eth is really wanted in specific configuration.

For stuff like that I'd make a function (to setup low-level stuff), and pass it 
via platform_info to the eth driver, so that really driver-specific things happen in driver context only.

Yes this is board specific file, and virtually everything needed for the board can take place here. 
But usually BCSR acts as a toggle for a several things, and IOW, I see it more correct to trigger those stuff from the respective drivers (using a callback passed through platform_info)

-Vitaly

Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Andy Fleming]

On Jun 29, 2006, at 13:51, Vitaly Bordug wrote:

> On Thu, 29 Jun 2006 13:03:23 -0500
> Andy Fleming wrote:
>
> [snip]
>>>>> +	iounmap(bcsr_regs);
>>>>> +
>>>> And if we have a design, which do not contain real ethernet UCC
>>>> usage? Or UCC
>>>> geth is disabled somehow explicitly? Stuff like that normally
>>>> goes to the
>>>> callback that is going to be triggered upon Etherbet init.
>>> I will move it.
>>
>>
>> Wait...no.  I don't understand Vitaly's objection.  If someone
>> creates a board with an 8360 that doesn't use the UCC ethernet, they
>> can create a separate board file.  This is the board-specific code,
>> and it is perfectly acceptable for it to reset the PHY like this.
>> What ethernet callback could be used?
>>
>
> I am sort of against the unconditional trigger of the ethernet- 
> specific stuff,
> dependless if UCC eth is really wanted in specific configuration.
>
> For stuff like that I'd make a function (to setup low-level stuff),  
> and pass it
> via platform_info to the eth driver, so that really driver-specific  
> things happen in driver context only.
>
> Yes this is board specific file, and virtually everything needed  
> for the board can take place here.
> But usually BCSR acts as a toggle for a several things, and IOW, I  
> see it more correct to trigger those stuff from the respective  
> drivers (using a callback passed through platform_info)


Callbacks are fairly evil.  And the driver most certainly cannot know  
about the BCSR, since there may or may not even *be* a BCSR on any  
given board with a QE.  The PHY only needs to be reset once, during  
initialization.  On some boards, there is no need to trigger some  
sort of reset, or enable any PHYs.  I'm still not sure why this  
should be the domain of the device driver, since it's a board option.

Andy

Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Vitaly Bordug]

On Thu, 29 Jun 2006 14:18:51 -0500
Andy Fleming wrote:

> 
> On Jun 29, 2006, at 13:51, Vitaly Bordug wrote:
> 
> > On Thu, 29 Jun 2006 13:03:23 -0500
> > Andy Fleming wrote:
> >
> > [snip]
> >>>>> +	iounmap(bcsr_regs);
> >>>>> +
> >>>> And if we have a design, which do not contain real ethernet UCC
> >>>> usage? Or UCC
> >>>> geth is disabled somehow explicitly? Stuff like that normally
> >>>> goes to the
> >>>> callback that is going to be triggered upon Etherbet init.
> >>> I will move it.
> >>
> >>
> >> Wait...no.  I don't understand Vitaly's objection.  If someone
> >> creates a board with an 8360 that doesn't use the UCC ethernet,
> >> they can create a separate board file.  This is the board-specific
> >> code, and it is perfectly acceptable for it to reset the PHY like
> >> this. What ethernet callback could be used?
> >>
> >
> > I am sort of against the unconditional trigger of the ethernet- 
> > specific stuff,
> > dependless if UCC eth is really wanted in specific configuration.
> >
> > For stuff like that I'd make a function (to setup low-level
> > stuff), and pass it
> > via platform_info to the eth driver, so that really
> > driver-specific things happen in driver context only.
> >
> > Yes this is board specific file, and virtually everything needed  
> > for the board can take place here.
> > But usually BCSR acts as a toggle for a several things, and IOW, I  
> > see it more correct to trigger those stuff from the respective  
> > drivers (using a callback passed through platform_info)
> 
> 
> Callbacks are fairly evil.  And the driver most certainly cannot
> know about the BCSR, since there may or may not even *be* a BCSR on
> any given board with a QE.  The PHY only needs to be reset once,
> during initialization.  On some boards, there is no need to trigger
> some sort of reset, or enable any PHYs.  I'm still not sure why this  
> should be the domain of the device driver, since it's a board option.
> 

Well. The driver does not need to know anything about bcsr. All it needs to do is to execute the function pointer filled in bsp code, if one exists (If nothing needs to be tweaked in bsp level for a driver, just no need to fill that function pointer). For instance, in PQ2 uart, usb and fcc need to be enabled by bcsr before could be actually utilized, so say fs_enet does all needed upon startup, without messing with board setup code.
The same does cpm uart... 

In case of this particular board, it is not that bad. But I dislike the concept to execute the code in common (for this board) path, not depending if UCC eth disabled in config explicitly.

-Vitaly

Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Andy Fleming]

On Jun 29, 2006, at 14:56, Vitaly Bordug wrote:

> On Thu, 29 Jun 2006 14:18:51 -0500
> Andy Fleming wrote:
>
>>
>> On Jun 29, 2006, at 13:51, Vitaly Bordug wrote:
>>
>>> On Thu, 29 Jun 2006 13:03:23 -0500
>>> Andy Fleming wrote:
>>>
>>> [snip]
>>>>>>> +	iounmap(bcsr_regs);
>>>>>>> +
>>>>>> And if we have a design, which do not contain real ethernet UCC
>>>>>> usage? Or UCC
>>>>>> geth is disabled somehow explicitly? Stuff like that normally
>>>>>> goes to the
>>>>>> callback that is going to be triggered upon Etherbet init.
>>>>> I will move it.
>>>>
>>>>
>>>> Wait...no.  I don't understand Vitaly's objection.  If someone
>>>> creates a board with an 8360 that doesn't use the UCC ethernet,
>>>> they can create a separate board file.  This is the board-specific
>>>> code, and it is perfectly acceptable for it to reset the PHY like
>>>> this. What ethernet callback could be used?
>>>>
>>>
>>> I am sort of against the unconditional trigger of the ethernet-
>>> specific stuff,
>>> dependless if UCC eth is really wanted in specific configuration.
>>>
>>> For stuff like that I'd make a function (to setup low-level
>>> stuff), and pass it
>>> via platform_info to the eth driver, so that really
>>> driver-specific things happen in driver context only.
>>>
>>> Yes this is board specific file, and virtually everything needed
>>> for the board can take place here.
>>> But usually BCSR acts as a toggle for a several things, and IOW, I
>>> see it more correct to trigger those stuff from the respective
>>> drivers (using a callback passed through platform_info)
>>
>>
>> Callbacks are fairly evil.  And the driver most certainly cannot
>> know about the BCSR, since there may or may not even *be* a BCSR on
>> any given board with a QE.  The PHY only needs to be reset once,
>> during initialization.  On some boards, there is no need to trigger
>> some sort of reset, or enable any PHYs.  I'm still not sure why this
>> should be the domain of the device driver, since it's a board option.
>>
>
> Well. The driver does not need to know anything about bcsr. All it  
> needs to do is to execute the function pointer filled in bsp code,  
> if one exists (If nothing needs to be tweaked in bsp level for a  
> driver, just no need to fill that function pointer). For instance,  
> in PQ2 uart, usb and fcc need to be enabled by bcsr before could be  
> actually utilized, so say fs_enet does all needed upon startup,  
> without messing with board setup code.
> The same does cpm uart...
>
> In case of this particular board, it is not that bad. But I dislike  
> the concept to execute the code in common (for this board) path,  
> not depending if UCC eth disabled in config explicitly.

Well, let me try to see if I understand the two approaches being  
pondered:

1) Use a callback.

Inside the platform info device-specific structure, we create a  
callback.  Something like enet_info->board_init().  The board boots,  
and in the initialization function for that board, the callback is  
assigned to be a function which does the appropriate board-specific  
initialization.  Actually, it makes sure to do this for every device  
which requires such initialization.  Then, later, the devices are  
registered, and matched up with appropriate drivers.  Those drivers  
make sure to invoke enet_info->board_init() before they do anything  
hw related.

2) Let board init code do it

The board boots, and in the initialization function for that board,  
it checks to see if the device exists (by searching the device tree),  
and if so, does any board-specific initialization (in this case,  
configuring the board register to enable the PHY for that device).   
The devices are registered, and matched with appropriate drivers.   
Those drivers operate, blissfully unaware that there was ever any  
danger the board wasn't set up.

Andy

Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Vitaly Bordug]

On Thu, 29 Jun 2006 16:04:21 -0500
Andy Fleming wrote:

> 
> On Jun 29, 2006, at 14:56, Vitaly Bordug wrote:
> 
> > On Thu, 29 Jun 2006 14:18:51 -0500
> > Andy Fleming wrote:
> >
> >>
> >> On Jun 29, 2006, at 13:51, Vitaly Bordug wrote:
> >>
> >>> On Thu, 29 Jun 2006 13:03:23 -0500
> >>> Andy Fleming wrote:
> >>>
> >>> [snip]
> >>>>>>> +	iounmap(bcsr_regs);
> >>>>>>> +
> >>>>>> And if we have a design, which do not contain real ethernet UCC
> >>>>>> usage? Or UCC
> >>>>>> geth is disabled somehow explicitly? Stuff like that normally
> >>>>>> goes to the
> >>>>>> callback that is going to be triggered upon Etherbet init.
> >>>>> I will move it.
> >>>>
> >>>>
> >>>> Wait...no.  I don't understand Vitaly's objection.  If someone
> >>>> creates a board with an 8360 that doesn't use the UCC ethernet,
> >>>> they can create a separate board file.  This is the
> >>>> board-specific code, and it is perfectly acceptable for it to
> >>>> reset the PHY like this. What ethernet callback could be used?
> >>>>
> >>>
> >>> I am sort of against the unconditional trigger of the ethernet-
> >>> specific stuff,
> >>> dependless if UCC eth is really wanted in specific configuration.
> >>>
> >>> For stuff like that I'd make a function (to setup low-level
> >>> stuff), and pass it
> >>> via platform_info to the eth driver, so that really
> >>> driver-specific things happen in driver context only.
> >>>
> >>> Yes this is board specific file, and virtually everything needed
> >>> for the board can take place here.
> >>> But usually BCSR acts as a toggle for a several things, and IOW, I
> >>> see it more correct to trigger those stuff from the respective
> >>> drivers (using a callback passed through platform_info)
> >>
> >>
> >> Callbacks are fairly evil.  And the driver most certainly cannot
> >> know about the BCSR, since there may or may not even *be* a BCSR on
> >> any given board with a QE.  The PHY only needs to be reset once,
> >> during initialization.  On some boards, there is no need to trigger
> >> some sort of reset, or enable any PHYs.  I'm still not sure why
> >> this should be the domain of the device driver, since it's a board
> >> option.
> >>
> >
> > Well. The driver does not need to know anything about bcsr. All it  
> > needs to do is to execute the function pointer filled in bsp code,  
> > if one exists (If nothing needs to be tweaked in bsp level for a  
> > driver, just no need to fill that function pointer). For instance,  
> > in PQ2 uart, usb and fcc need to be enabled by bcsr before could
> > be actually utilized, so say fs_enet does all needed upon startup,  
> > without messing with board setup code.
> > The same does cpm uart...
> >
> > In case of this particular board, it is not that bad. But I
> > dislike the concept to execute the code in common (for this board)
> > path, not depending if UCC eth disabled in config explicitly.
> 
> Well, let me try to see if I understand the two approaches being  
> pondered:
> 
Yes, just right.

> 1) Use a callback.
> 
> Inside the platform info device-specific structure, we create a  
> callback.  Something like enet_info->board_init().  The board boots,  
> and in the initialization function for that board, the callback is  
> assigned to be a function which does the appropriate board-specific  
> initialization.  Actually, it makes sure to do this for every device  
> which requires such initialization.  Then, later, the devices are  
> registered, and matched up with appropriate drivers.  Those drivers  
> make sure to invoke enet_info->board_init() before they do anything  
> hw related.
> 
> 2) Let board init code do it
> 
> The board boots, and in the initialization function for that board,  
> it checks to see if the device exists (by searching the device
> tree), and if so, does any board-specific initialization (in this
> case, configuring the board register to enable the PHY for that
> device). The devices are registered, and matched with appropriate
> drivers. Those drivers operate, blissfully unaware that there was
> ever any danger the board wasn't set up.
> 

Sounds fine, but there are some corner cases. 
In case, really familiar to 8xx people, the board actually has devices, but 
they simply do not operate simultaneously (because of hw, or there are conflicting pin options)

So the only way to work in such a case is to craft proper kconfig for say, secondary Eth off, 2-nd uart on and vice versa.  BSP code could be aware of that, and make/do not make hw tweaks up to #ifdefs. The way for BSP code to put needed stuff into the function, hereby telling the driver to execute it upon setup before accessing hw seems more consistent way for me. 

Again, I agree it may be extra for this particular board. But we are speaking about the concept... That sort of things is used within fs_enet and cpm_uart drivers already in the stock tree.

-Vitaly

RE: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Li Yang-r58472]

Both of you are reasonable.  But the simplest fix goes like this:

-       /* Reset the Ethernet PHY */
-       bcsr_regs = (u8 *) ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
-       bcsr_regs[9] &= ~0x20;
-       udelay(1000);
-       bcsr_regs[9] |= 0x20;
-       iounmap(bcsr_regs);
+       if ((np = of_find_compatible_node(NULL, "network", "ucc_geth"))
+                       != NULL){
+               /* Reset the Ethernet PHY */
+               bcsr_regs = (u8 *) ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
+               bcsr_regs[9] &= ~0x20;
+               udelay(1000);
+               bcsr_regs[9] |= 0x20;
+               iounmap(bcsr_regs);
+       }


> -----Original Message-----
> From: Vitaly Bordug [mailto:vbordug@ru.mvista.com]
> Sent: Friday, June 30, 2006 6:10 AM
> To: Fleming Andy-afleming
> Cc: Li Yang-r58472; Yin Olivia-r63875; linux-kernel@vger.kernel.org Mailing
> List; linuxppc-dev list; Chu hanjin-r52514
> Subject: Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support
> 
> On Thu, 29 Jun 2006 16:04:21 -0500
> Andy Fleming wrote:
> 
> >
> > On Jun 29, 2006, at 14:56, Vitaly Bordug wrote:
> >
> > > On Thu, 29 Jun 2006 14:18:51 -0500
> > > Andy Fleming wrote:
> > >
> > >>
> > >> On Jun 29, 2006, at 13:51, Vitaly Bordug wrote:
> > >>
> > >>> On Thu, 29 Jun 2006 13:03:23 -0500
> > >>> Andy Fleming wrote:
> > >>>
> > >>> [snip]
> > >>>>>>> +	iounmap(bcsr_regs);
> > >>>>>>> +
> > >>>>>> And if we have a design, which do not contain real ethernet UCC
> > >>>>>> usage? Or UCC
> > >>>>>> geth is disabled somehow explicitly? Stuff like that normally
> > >>>>>> goes to the
> > >>>>>> callback that is going to be triggered upon Etherbet init.
> > >>>>> I will move it.
> > >>>>
> > >>>>
> > >>>> Wait...no.  I don't understand Vitaly's objection.  If someone
> > >>>> creates a board with an 8360 that doesn't use the UCC ethernet,
> > >>>> they can create a separate board file.  This is the
> > >>>> board-specific code, and it is perfectly acceptable for it to
> > >>>> reset the PHY like this. What ethernet callback could be used?
> > >>>>
> > >>>
> > >>> I am sort of against the unconditional trigger of the ethernet-
> > >>> specific stuff,
> > >>> dependless if UCC eth is really wanted in specific configuration.
> > >>>
> > >>> For stuff like that I'd make a function (to setup low-level
> > >>> stuff), and pass it
> > >>> via platform_info to the eth driver, so that really
> > >>> driver-specific things happen in driver context only.
> > >>>
> > >>> Yes this is board specific file, and virtually everything needed
> > >>> for the board can take place here.
> > >>> But usually BCSR acts as a toggle for a several things, and IOW, I
> > >>> see it more correct to trigger those stuff from the respective
> > >>> drivers (using a callback passed through platform_info)
> > >>
> > >>
> > >> Callbacks are fairly evil.  And the driver most certainly cannot
> > >> know about the BCSR, since there may or may not even *be* a BCSR on
> > >> any given board with a QE.  The PHY only needs to be reset once,
> > >> during initialization.  On some boards, there is no need to trigger
> > >> some sort of reset, or enable any PHYs.  I'm still not sure why
> > >> this should be the domain of the device driver, since it's a board
> > >> option.
> > >>
> > >
> > > Well. The driver does not need to know anything about bcsr. All it
> > > needs to do is to execute the function pointer filled in bsp code,
> > > if one exists (If nothing needs to be tweaked in bsp level for a
> > > driver, just no need to fill that function pointer). For instance,
> > > in PQ2 uart, usb and fcc need to be enabled by bcsr before could
> > > be actually utilized, so say fs_enet does all needed upon startup,
> > > without messing with board setup code.
> > > The same does cpm uart...
> > >
> > > In case of this particular board, it is not that bad. But I
> > > dislike the concept to execute the code in common (for this board)
> > > path, not depending if UCC eth disabled in config explicitly.
> >
> > Well, let me try to see if I understand the two approaches being
> > pondered:
> >
> Yes, just right.
> 
> > 1) Use a callback.
> >
> > Inside the platform info device-specific structure, we create a
> > callback.  Something like enet_info->board_init().  The board boots,
> > and in the initialization function for that board, the callback is
> > assigned to be a function which does the appropriate board-specific
> > initialization.  Actually, it makes sure to do this for every device
> > which requires such initialization.  Then, later, the devices are
> > registered, and matched up with appropriate drivers.  Those drivers
> > make sure to invoke enet_info->board_init() before they do anything
> > hw related.
> >
> > 2) Let board init code do it
> >
> > The board boots, and in the initialization function for that board,
> > it checks to see if the device exists (by searching the device
> > tree), and if so, does any board-specific initialization (in this
> > case, configuring the board register to enable the PHY for that
> > device). The devices are registered, and matched with appropriate
> > drivers. Those drivers operate, blissfully unaware that there was
> > ever any danger the board wasn't set up.
> >
> 
> Sounds fine, but there are some corner cases.
> In case, really familiar to 8xx people, the board actually has devices, but
> they simply do not operate simultaneously (because of hw, or there are
> conflicting pin options)
> 
> So the only way to work in such a case is to craft proper kconfig for say,
> secondary Eth off, 2-nd uart on and vice versa.  BSP code could be aware of
> that, and make/do not make hw tweaks up to #ifdefs. The way for BSP code to
> put needed stuff into the function, hereby telling the driver to execute it
> upon setup before accessing hw seems more consistent way for me.
> 
> Again, I agree it may be extra for this particular board. But we are speaking
> about the concept... That sort of things is used within fs_enet and cpm_uart
> drivers already in the stock tree.
> 
> -Vitaly

RE: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Li Yang-r58472]

> -----Original Message-----
> From: Vitaly Bordug [mailto:vbordug@ru.mvista.com]
> Sent: Thursday, June 29, 2006 12:59 AM
> To: Li Yang-r58472
> Cc: 'Paul Mackerras'; linuxppc-dev@ozlabs.org; Phillips Kim-R1AAHA; Chu
> hanjin-r52514; Yin Olivia-r63875; 'linux-kernel@vger.kernel.org'
> Subject: Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support
> 
> On Wed, 28 Jun 2006 22:23:03 +0800
> Li Yang-r58472 <LeoLi@freescale.com> wrote:
> 
[snip]
> 
> >
> >  config MPC834x
> > @@ -24,4 +31,10 @@ config MPC834x
> >  	select PPC_INDIRECT_PCI
> >  	default y if MPC834x_SYS
> >
> > +config MPC836x
> > +	bool
> > +	select PPC_UDBG_16550
> 
> debug option made default?

I'm afraid this is needed to boot.  83xx family platforms need it to initialize early console.  And it does appear in several defconfigs of other platforms.
> > +	select PPC_INDIRECT_PCI
> > +	default y if MPC8360E_PB
> > +
> >  endmenu

RE: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Benjamin Herrenschmidt]

On Fri, 2006-06-30 at 18:27 +0800, Li Yang-r58472 wrote:
> > -----Original Message-----
> > From: Vitaly Bordug [mailto:vbordug@ru.mvista.com]
> > Sent: Thursday, June 29, 2006 12:59 AM
> > To: Li Yang-r58472
> > Cc: 'Paul Mackerras'; linuxppc-dev@ozlabs.org; Phillips Kim-R1AAHA; Chu
> > hanjin-r52514; Yin Olivia-r63875; 'linux-kernel@vger.kernel.org'
> > Subject: Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support
> > 
> > On Wed, 28 Jun 2006 22:23:03 +0800
> > Li Yang-r58472 <LeoLi@freescale.com> wrote:
> > 
> [snip]
> > 
> > >
> > >  config MPC834x
> > > @@ -24,4 +31,10 @@ config MPC834x
> > >  	select PPC_INDIRECT_PCI
> > >  	default y if MPC834x_SYS
> > >
> > > +config MPC836x
> > > +	bool
> > > +	select PPC_UDBG_16550
> > 
> > debug option made default?
> 
> I'm afraid this is needed to boot.  83xx family platforms need it to initialize early console.  And it does appear in several defconfigs of other platforms.

How so ? Why would having a serial console be mandatory ? Embedded might
want to boot without a console and use the serial port for other things.
This should be left as a config option (though you are welcome to put it
in the defconfig for your platform)
 
> > > +	select PPC_INDIRECT_PCI
> > > +	default y if MPC8360E_PB
> > > +
> > >  endmenu
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Li Yang-r58472]

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Saturday, July 01, 2006 5:00 PM
> To: Li Yang-r58472
> Cc: 'Vitaly Bordug'; 'Paul Mackerras'; linuxppc-dev@ozlabs.org; Phillips
> Kim-R1AAHA; Chu hanjin-r52514; Yin Olivia-r63875;
> 'linux-kernel@vger.kernel.org'
> Subject: RE: [PATCH 1/7] powerpc: Add mpc8360epb platform support
> 
> On Fri, 2006-06-30 at 18:27 +0800, Li Yang-r58472 wrote:
> > > -----Original Message-----
> > > From: Vitaly Bordug [mailto:vbordug@ru.mvista.com]
> > > Sent: Thursday, June 29, 2006 12:59 AM
> > > To: Li Yang-r58472
> > > Cc: 'Paul Mackerras'; linuxppc-dev@ozlabs.org; Phillips Kim-R1AAHA; Chu
> > > hanjin-r52514; Yin Olivia-r63875; 'linux-kernel@vger.kernel.org'
> > > Subject: Re: [PATCH 1/7] powerpc: Add mpc8360epb platform support
> > >
> > > On Wed, 28 Jun 2006 22:23:03 +0800
> > > Li Yang-r58472 <LeoLi@freescale.com> wrote:
> > >
> > [snip]
> > >
> > > >
> > > >  config MPC834x
> > > > @@ -24,4 +31,10 @@ config MPC834x
> > > >  	select PPC_INDIRECT_PCI
> > > >  	default y if MPC834x_SYS
> > > >
> > > > +config MPC836x
> > > > +	bool
> > > > +	select PPC_UDBG_16550
> > >
> > > debug option made default?
> >
> > I'm afraid this is needed to boot.  83xx family platforms need it to
> initialize early console.  And it does appear in several defconfigs of other
> platforms.
> 
> How so ? Why would having a serial console be mandatory ? Embedded might
> want to boot without a console and use the serial port for other things.
> This should be left as a config option (though you are welcome to put it
> in the defconfig for your platform)

The problem is that we don't have this PPC_UBDG_16550 option configurable now, and it is only made by default for several boards including pseries, chrp, prep, cell, and all 83xx and 85xx platforms.  If we need to change the whole thing, how should we do it?
> 
> > > > +	select PPC_INDIRECT_PCI
> > > > +	default y if MPC8360E_PB
> > > > +
> > > >  endmenu
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/7] powerpc: Add mpc8360epb platform support

[Benjamin Herrenschmidt]

> The problem is that we don't have this PPC_UBDG_16550 option configurable now,
> and it is only made by default for several boards including pseries, chrp, prep, cell,
> and all 83xx and 85xx platforms.  If we need to change the whole thing, how should we do it?

It's up to your platformn code to enable it though. udbg will only kick
in from the legacy serial console code if your firmware indicates that
stdout is on a serial port. If the firmware doesn't, udbg won't kick in.
But yeah, build it in by default if you want then.

Ben.