* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 18:14 ` [PATCH 1/4] " Josh Boyer
@ 2007-05-04 18:35 ` Kumar Gala
2007-05-04 19:11 ` Josh Boyer
2007-05-04 19:01 ` Olof Johansson
2007-05-04 19:44 ` Arnd Bergmann
2 siblings, 1 reply; 25+ messages in thread
From: Kumar Gala @ 2007-05-04 18:35 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
On May 4, 2007, at 1:14 PM, Josh Boyer wrote:
> Add PowerPC 750 Holly/Hickory platform support
>
> Signed-off-by: Stephen Winiecki <stevewin@us.ibm.com>
> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> ---
> arch/powerpc/Kconfig | 2
> arch/powerpc/platforms/embedded6xx/Kconfig | 12 -
> arch/powerpc/platforms/embedded6xx/Makefile | 1
> arch/powerpc/platforms/embedded6xx/holly.c | 326 ++++++++++++++++
> ++++++++++++
> arch/powerpc/platforms/embedded6xx/holly.h | 24 ++
> drivers/net/tsi108_eth.h | 4
> include/asm-powerpc/tsi108.h | 4
> 7 files changed, 371 insertions(+), 2 deletions(-)
>
> --- linux-2.6.orig/arch/powerpc/Kconfig
> +++ linux-2.6/arch/powerpc/Kconfig
> @@ -659,7 +659,7 @@ config MCA
> config PCI
> bool "PCI support" if 40x || CPM2 || PPC_83xx || PPC_85xx ||
> PPC_86xx \
> || PPC_MPC52xx || (EMBEDDED && (PPC_PSERIES || PPC_ISERIES)) \
> - || MPC7448HPC2 || PPC_PS3
> + || MPC7448HPC2 || PPC_PS3 || HOLLY
> default y if !40x && !CPM2 && !8xx && !APUS && !PPC_83xx \
> && !PPC_85xx && !PPC_86xx
> default PCI_PERMEDIA if !4xx && !CPM2 && !8xx && APUS
> --- linux-2.6.orig/arch/powerpc/platforms/embedded6xx/Kconfig
> +++ linux-2.6/arch/powerpc/platforms/embedded6xx/Kconfig
> @@ -25,11 +25,21 @@ config MPC7448HPC2
> help
> Select MPC7448HPC2 if configuring for Freescale MPC7448HPC2
> (Taiga)
> platform
> +
> +config HOLLY
> + bool "PPC750GX/CL with TSI10x bridge (Hickory/Holly)"
> + select TSI108_BRIDGE
> + select PPC_UDBG_16550
> + select MPIC
> + select MPIC_WEIRD
> + help
> + Select HOLLY if configuring for an IBM 750GX/CL Eval
> + Board with TSI108/9 bridge (Hickory/Holly)
> endchoice
>
> config TSI108_BRIDGE
> bool
I wondering if we should have select MPIC and MPIC_WEIRD here instead
of on the board config.
> - depends on MPC7448HPC2
> + depends on MPC7448HPC2 || HOLLY
> default y
>
> config MPC10X_BRIDGE
> --- linux-2.6.orig/arch/powerpc/platforms/embedded6xx/Makefile
> +++ linux-2.6/arch/powerpc/platforms/embedded6xx/Makefile
> @@ -3,3 +3,4 @@
> #
> obj-$(CONFIG_MPC7448HPC2) += mpc7448_hpc2.o
> obj-$(CONFIG_LINKSTATION) += linkstation.o ls_uart.o
> +obj-$(CONFIG_HOLLY) += holly.o
> --- /dev/null
> +++ linux-2.6/arch/powerpc/platforms/embedded6xx/holly.c
> @@ -0,0 +1,326 @@
> +/*
> + * Board setup routines for the IBM 750GX/CL platform w/ TSI10x
> bridge
> + *
> + * Copyright 2007 IBM Corporation
> + *
> + * Stephen Winiecki <stevewin@us.ibm.com>
> + * Josh Boyer <jwboyer@linux.vnet.ibm.com>
> + *
> + * Based on code from mpc7448_hpc2.c
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + */
> +
> +#include <linux/stddef.h>
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +#include <linux/kdev_t.h>
> +#include <linux/console.h>
> +#include <linux/delay.h>
> +#include <linux/irq.h>
> +#include <linux/ide.h>
> +#include <linux/seq_file.h>
> +#include <linux/root_dev.h>
> +#include <linux/serial.h>
> +#include <linux/tty.h>
> +#include <linux/serial_core.h>
> +
> +#include <asm/system.h>
> +#include <asm/time.h>
> +#include <asm/machdep.h>
> +#include <asm/prom.h>
> +#include <asm/udbg.h>
> +#include <asm/tsi108.h>
> +#include <asm/pci-bridge.h>
> +#include <asm/reg.h>
> +#include <mm/mmu_decl.h>
> +#include "holly.h"
> +#include <asm/tsi108_irq.h>
> +#include <asm/mpic.h>
> +#include <asm/of_platform.h>
> +
> +#undef DEBUG
> +#ifdef DEBUG
> +#define DBG(fmt...) do { printk(fmt); } while(0)
> +#else
> +#define DBG(fmt...) do { } while(0)
> +#endif
> +
> +#ifndef CONFIG_PCI
> +pci_dram_offset = PPC750GXCL_TSI_PCI_MEM_OFFSET;
> +#endif
> +
> +extern int tsi108_setup_pci(struct device_node *dev);
> +extern void tsi108_pci_int_init(struct device_node *node);
> +extern void tsi108_irq_cascade(unsigned int irq, struct irq_desc
> *desc);
> +
> +int holly_exclude_device(u_char bus, u_char devfn)
> +{
> + if (bus == 0 && PCI_SLOT(devfn) == 0)
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + else
> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static void holly_remap_bridge(void)
> +{
> + u32 lut_val, lut_addr = 0x900, misc_cfg;
> + int i;
> +
> + printk(KERN_ERR "Remapping PCI bridge\n");
> +
> + /* Re-init the PCI bridge and LUT registers to have mappings that
> don't
> + * rely on PIBS */
> + for (i = 0; i < 31; i++) {
> + tsi108_write_reg(TSI108_PB_OFFSET + lut_addr, 0x00000201);
> + lut_addr += 4;
> + tsi108_write_reg(TSI108_PB_OFFSET + lut_addr, 0x0);
> + lut_addr += 4;
> + }
> +
> + /* Reserve the last LUT entry for PCI I/O space */
> + tsi108_write_reg(TSI108_PB_OFFSET + lut_addr, 0x00000241);
> + lut_addr += 4;
> + tsi108_write_reg(TSI108_PB_OFFSET + lut_addr, 0x0);
> +
> + /* Map PCI I/O space */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x210, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x20c, 0x1);
> +
> + /* Map PCI CFG space */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x208, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x204, 0x7c000000 | 0x01);
> +
> + /* We don't need MEM32 and PRM remapping so disable them */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x214, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x220, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x230, 0x0);
> +
> + /* Set P2O_BAR0 */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x14, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x10, 0xc0000000);
> +
> + /* Init the PCI LUTs to do no remapping */
> + lut_addr = 0x500;
> + lut_val = 0x00000002;
> +
> + for (i = 0; i < 32; i++) {
> + tsi108_write_reg(TSI108_PCI_OFFSET + lut_addr, lut_val);
> + lut_addr += 4;
> + tsi108_write_reg(TSI108_PCI_OFFSET + lut_addr, 0x40000000);
> + lut_addr += 4;
> + lut_val += 0x02000000;
> + }
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x4c, 0x00007900);
> +
> + /* Set 64-bit PCI bus address for system memory */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x1c, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x18, 0x0);
> +}
> +
> +static void __init holly_setup_arch(void)
> +{
> + struct device_node *cpu;
> + struct device_node *np;
> +
> + if (ppc_md.progress)
> + ppc_md.progress("holly_setup_arch():set_bridge", 0);
> +
> + cpu = of_find_node_by_type(NULL, "cpu");
> + if (cpu != 0) {
> + const unsigned int *fp;
> +
> + fp = of_get_property(cpu, "clock-frequency", NULL);
> + if (fp != 0)
> + loops_per_jiffy = *fp / HZ;
> + else
> + loops_per_jiffy = 50000000 / HZ;
> + of_node_put(cpu);
> + }
> + tsi108_csr_vir_base = get_vir_csrbase();
> +
> + /* setup PCI host bridge */
> + holly_remap_bridge();
> +#ifdef CONFIG_PCI
> + for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
> + tsi108_setup_pci(np);
> +
> + ppc_md.pci_exclude_device = holly_exclude_device;
> + if (ppc_md.progress)
> + ppc_md.progress("tsi108: resources set", 0x100);
> +#endif
> + printk(KERN_INFO "PPC750GX/CL Platform\n");
Should this be something like "Holly PPC750GX/CL Platform"
> +}
> +
> +/*
> + * Interrupt setup and service. Interrrupts on the holly come
> + * from the four external INT pins, PCI interrupts are routed via
> + * PCI interrupt control registers, it generates internal IRQ23
> + *
> + * Interrupt routing on the Holly Board:
> + * TSI108:PB_INT[0] -> CPU0:INT#
> + * TSI108:PB_INT[1] -> CPU0:MCP#
> + * TSI108:PB_INT[2] -> N/C
> + * TSI108:PB_INT[3] -> N/C
> + */
> +static void __init holly_init_IRQ(void)
> +{
> + struct mpic *mpic;
> + phys_addr_t mpic_paddr = 0;
> + struct device_node *tsi_pic;
> +#ifdef CONFIG_PCI
> + unsigned int cascade_pci_irq;
> + struct device_node *tsi_pci;
> + struct device_node *cascade_node = NULL;
> +#endif
> +
> + tsi_pic = of_find_node_by_type(NULL, "open-pic");
> + if (tsi_pic) {
> + unsigned int size;
> + const void *prop = of_get_property(tsi_pic, "reg", &size);
> + mpic_paddr = of_translate_address(tsi_pic, prop);
> + }
> +
> + if (mpic_paddr == 0) {
> + printk(KERN_ERR "%s: No tsi108 PIC found !\n", __func__);
> + return;
> + }
> +
> + DBG("%s: tsi108 pic phys_addr = 0x%x\n", __func__, (u32)
> mpic_paddr);
> +
> + mpic = mpic_alloc(tsi_pic, mpic_paddr,
> + MPIC_PRIMARY | MPIC_BIG_ENDIAN | MPIC_WANTS_RESET |
> + MPIC_SPV_EOI | MPIC_NO_PTHROU_DIS | MPIC_REGSET_TSI108,
> + 24,
> + NR_IRQS-4, /* num_sources used */
> + "Tsi108_PIC");
> +
> + BUG_ON(mpic == NULL);
> +
> + mpic_assign_isu(mpic, 0, mpic_paddr + 0x100);
> +
> + mpic_init(mpic);
> +
> +#ifdef CONFIG_PCI
> + tsi_pci = of_find_node_by_type(NULL, "pci");
> + if (tsi_pci == NULL) {
> + printk(KERN_ERR "%s: No tsi108 pci node found !\n", __func__);
> + return;
> + }
> +
> + cascade_node = of_find_node_by_type(NULL, "pic-router");
> + if (cascade_node == NULL) {
> + printk(KERN_ERR "%s: No tsi108 pci cascade node found !\n",
> __func__);
> + return;
> + }
> +
> + cascade_pci_irq = irq_of_parse_and_map(tsi_pci, 0);
> + DBG("%s: tsi108 cascade_pci_irq = 0x%x\n", __func__, (u32)
> cascade_pci_irq);
> + tsi108_pci_int_init(cascade_node);
> + set_irq_data(cascade_pci_irq, mpic);
> + set_irq_chained_handler(cascade_pci_irq, tsi108_irq_cascade);
> +#endif
> + /* Configure MPIC outputs to CPU0 */
> + tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0);
> + of_node_put(tsi_pic);
> +}
> +
> +void holly_show_cpuinfo(struct seq_file *m)
> +{
> + seq_printf(m, "vendor\t\t: IBM\n");
> + seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
Should 'Holly' be in the machine name?
> +}
> +
> +void holly_restart(char *cmd)
> +{
> + unsigned long *ocn_bar1 = NULL;
This should probably be __be32 __iomem
> + unsigned long bar;
> + struct device_node *bridge = NULL;
> + const void *prop;
> + int size;
> + phys_addr_t addr = 0xc0000000;
> +
> + local_irq_disable();
> +
> + bridge = of_find_node_by_type(NULL, "tsi-bridge");
> + if (bridge) {
> + prop = of_get_property(bridge, "reg", &size);
> + addr = of_translate_address(bridge, prop);
> + }
> + addr += (TSI108_PB_OFFSET + 0x414);
> +
> + ocn_bar1 = ioremap(addr, 0x4);
> +
> + /* Turn on the BOOT bit so the addresses are correctly
> + * routed to the HLP interface */
> + bar = ioread32be(ocn_bar1);
> + bar |= 2;
> + iowrite32be(bar, ocn_bar1);
> + iosync();
> +
> + /* Set SRR0 to the reset vector and turn on MSR_IP */
> + mtspr(SPRN_SRR0, 0xfff00100);
> + mtspr(SPRN_SRR1, MSR_IP);
> +
> + /* Do an rfi to jump back to firmware. Somewhat evil,
> + * but it works
> + */
> + __asm__ __volatile__("rfi" : : : "memory");
> +
> + /* Spin until reset happens. Shouldn't really get here */
> + for (;;) ;
> +}
> +
> +void holly_power_off(void)
> +{
> + local_irq_disable();
> + /* No way to shut power off with software */
> + for (;;) ;
> +}
> +
> +void holly_halt(void)
> +{
> + holly_power_off();
> +}
> +
> +/*
> + * Called very early, device-tree isn't unflattened
> + */
> +static int __init holly_probe(void)
> +{
> + unsigned long root = of_get_flat_dt_root();
> +
> + if (!of_flat_dt_is_compatible(root, "ppc750"))
> + return 0;
This seems like to generic of a match.
> + return 1;
> +}
> +
> +static int ppc750_machine_check_exception(struct pt_regs *regs)
> +{
> + extern void tsi108_clear_pci_cfg_error(void);
> + const struct exception_table_entry *entry;
> +
> + /* Are we prepared to handle this fault */
> + if ((entry = search_exception_tables(regs->nip)) != NULL) {
> + tsi108_clear_pci_cfg_error();
> + regs->msr |= MSR_RI;
> + regs->nip = entry->fixup;
> + return 1;
> + }
> + return 0;
> +}
> +
> +define_machine(holly){
> + .name = "PPC750 GX/CL TSI",
> + .probe = holly_probe,
> + .setup_arch = holly_setup_arch,
> + .init_IRQ = holly_init_IRQ,
> + .show_cpuinfo = holly_show_cpuinfo,
> + .get_irq = mpic_get_irq,
> + .restart = holly_restart,
> + .calibrate_decr = generic_calibrate_decr,
> + .machine_check_exception = ppc750_machine_check_exception,
> + .progress = udbg_progress,
> +};
> --- /dev/null
> +++ linux-2.6/arch/powerpc/platforms/embedded6xx/holly.h
> @@ -0,0 +1,24 @@
> +/*
> + * Definitions for the IBM 750GX/CL platform w/ TSI10x bridge
> + *
> + * Copyright 2007 IBM Corporation
> + *
> + * Stephen Winiecki <stevewin@us.ibm.com>
> + * Josh Boyer <jwboyer@linux.vnet.ibm.com>
> + *
> + * Based on code from mpc7448_hpc2.h
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + */
> +
> +#ifndef __PPC_PLATFORMS_PPC750GXCL_TSI_H
> +#define __PPC_PLATFORMS_PPC750GXCL_TSI_H
> +
> +#include <asm/ppcboot.h>
> +
> +/* Base Addresses for the PCI bus
> + */
> +#define PPC750GXCL_TSI_PCI_MEM_OFFSET (0x00000000)
> +#endif /* __PPC_PLATFORMS_H */
> --- linux-2.6.orig/drivers/net/tsi108_eth.h
> +++ linux-2.6/drivers/net/tsi108_eth.h
> @@ -49,7 +49,11 @@
> */
> #define PHY_MV88E 1 /* Marvel 88Exxxx PHY */
> #define PHY_BCM54XX 2 /* Broardcom BCM54xx PHY */
> +#if defined(CONFIG_HOLLY)
> +#define TSI108_PHY_TYPE PHY_BCM54XX
> +#else
> #define TSI108_PHY_TYPE PHY_MV88E
> +#endif
This seems pretty bad. Can we at least make this some type of kernel
config since I'm guessing the tsi108 isn't using the phylib.
> /*
> * TSI108 GIGE port registers
> --- linux-2.6.orig/include/asm-powerpc/tsi108.h
> +++ linux-2.6/include/asm-powerpc/tsi108.h
> @@ -68,7 +68,11 @@
> #define TSI108_PB_ERRCS_ES (1 << 1)
> #define TSI108_PB_ISR_PBS_RD_ERR (1 << 8)
>
> +#ifdef CONFIG_HOLLY
> +#define TSI108_PCI_CFG_BASE_PHYS (0x7c000000)
> +#else
> #define TSI108_PCI_CFG_BASE_PHYS (0xfb000000)
> +#endif
> #define TSI108_PCI_CFG_SIZE (0x01000000)
> /* Global variables */
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 18:35 ` Kumar Gala
@ 2007-05-04 19:11 ` Josh Boyer
0 siblings, 0 replies; 25+ messages in thread
From: Josh Boyer @ 2007-05-04 19:11 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
On Fri, 2007-05-04 at 13:35 -0500, Kumar Gala wrote:
> On May 4, 2007, at 1:14 PM, Josh Boyer wrote:
> > +
> > +config HOLLY
> > + bool "PPC750GX/CL with TSI10x bridge (Hickory/Holly)"
> > + select TSI108_BRIDGE
> > + select PPC_UDBG_16550
> > + select MPIC
> > + select MPIC_WEIRD
> > + help
> > + Select HOLLY if configuring for an IBM 750GX/CL Eval
> > + Board with TSI108/9 bridge (Hickory/Holly)
> > endchoice
> >
> > config TSI108_BRIDGE
> > bool
>
> I wondering if we should have select MPIC and MPIC_WEIRD here instead
> of on the board config.
Could be done, yes. I don't have an mpc7448hpc2 to test with, but I can
make that change.
> > + printk(KERN_INFO "PPC750GX/CL Platform\n");
>
> Should this be something like "Holly PPC750GX/CL Platform"
Well... no. It should really be "Hickory/Holly PPC750GX/CL Platform" to
be correct. But that sucks. Suppose I could match off of the cpu node
in the DT and just print one or the other... except they share a DT too
at the moment.
I'll think about what to do.
> > +void holly_show_cpuinfo(struct seq_file *m)
> > +{
> > + seq_printf(m, "vendor\t\t: IBM\n");
> > + seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
>
> Should 'Holly' be in the machine name?
Again, the whole "Hickory/Holly" thing.
>
> > +}
> > +
> > +void holly_restart(char *cmd)
> > +{
> > + unsigned long *ocn_bar1 = NULL;
>
> This should probably be __be32 __iomem
Good catch. I need to run this through sparse.
> > +/*
> > + * Called very early, device-tree isn't unflattened
> > + */
> > +static int __init holly_probe(void)
> > +{
> > + unsigned long root = of_get_flat_dt_root();
> > +
> > + if (!of_flat_dt_is_compatible(root, "ppc750"))
> > + return 0;
>
> This seems like to generic of a match.
Likely is, yes.
> > --- linux-2.6.orig/drivers/net/tsi108_eth.h
> > +++ linux-2.6/drivers/net/tsi108_eth.h
> > @@ -49,7 +49,11 @@
> > */
> > #define PHY_MV88E 1 /* Marvel 88Exxxx PHY */
> > #define PHY_BCM54XX 2 /* Broardcom BCM54xx PHY */
> > +#if defined(CONFIG_HOLLY)
> > +#define TSI108_PHY_TYPE PHY_BCM54XX
> > +#else
> > #define TSI108_PHY_TYPE PHY_MV88E
> > +#endif
>
> This seems pretty bad. Can we at least make this some type of kernel
> config since I'm guessing the tsi108 isn't using the phylib.
Yep.
Thanks for the comments. I'll fix it up in the next round.
josh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 18:14 ` [PATCH 1/4] " Josh Boyer
2007-05-04 18:35 ` Kumar Gala
@ 2007-05-04 19:01 ` Olof Johansson
2007-05-04 19:18 ` Josh Boyer
2007-05-04 19:44 ` Arnd Bergmann
2 siblings, 1 reply; 25+ messages in thread
From: Olof Johansson @ 2007-05-04 19:01 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
Hi,
Overall pretty good, I have a few comments below.
-Olof
On Fri, May 04, 2007 at 01:14:29PM -0500, Josh Boyer wrote:
> Add PowerPC 750 Holly/Hickory platform support
>
> Signed-off-by: Stephen Winiecki <stevewin@us.ibm.com>
> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> ---
> arch/powerpc/Kconfig | 2
> arch/powerpc/platforms/embedded6xx/Kconfig | 12 -
> arch/powerpc/platforms/embedded6xx/Makefile | 1
> arch/powerpc/platforms/embedded6xx/holly.c | 326 ++++++++++++++++++++++++++++
> arch/powerpc/platforms/embedded6xx/holly.h | 24 ++
> drivers/net/tsi108_eth.h | 4
> include/asm-powerpc/tsi108.h | 4
> 7 files changed, 371 insertions(+), 2 deletions(-)
>
> --- linux-2.6.orig/arch/powerpc/Kconfig
> +++ linux-2.6/arch/powerpc/Kconfig
> @@ -659,7 +659,7 @@ config MCA
> config PCI
> bool "PCI support" if 40x || CPM2 || PPC_83xx || PPC_85xx || PPC_86xx \
> || PPC_MPC52xx || (EMBEDDED && (PPC_PSERIES || PPC_ISERIES)) \
> - || MPC7448HPC2 || PPC_PS3
> + || MPC7448HPC2 || PPC_PS3 || HOLLY
> default y if !40x && !CPM2 && !8xx && !APUS && !PPC_83xx \
> && !PPC_85xx && !PPC_86xx
> default PCI_PERMEDIA if !4xx && !CPM2 && !8xx && APUS
> --- linux-2.6.orig/arch/powerpc/platforms/embedded6xx/Kconfig
> +++ linux-2.6/arch/powerpc/platforms/embedded6xx/Kconfig
> @@ -25,11 +25,21 @@ config MPC7448HPC2
> help
> Select MPC7448HPC2 if configuring for Freescale MPC7448HPC2 (Taiga)
> platform
> +
> +config HOLLY
> + bool "PPC750GX/CL with TSI10x bridge (Hickory/Holly)"
> + select TSI108_BRIDGE
> + select PPC_UDBG_16550
> + select MPIC
> + select MPIC_WEIRD
> + help
> + Select HOLLY if configuring for an IBM 750GX/CL Eval
> + Board with TSI108/9 bridge (Hickory/Holly)
> endchoice
>
> config TSI108_BRIDGE
> bool
> - depends on MPC7448HPC2
> + depends on MPC7448HPC2 || HOLLY
> default y
>
> config MPC10X_BRIDGE
> --- linux-2.6.orig/arch/powerpc/platforms/embedded6xx/Makefile
> +++ linux-2.6/arch/powerpc/platforms/embedded6xx/Makefile
> @@ -3,3 +3,4 @@
> #
> obj-$(CONFIG_MPC7448HPC2) += mpc7448_hpc2.o
> obj-$(CONFIG_LINKSTATION) += linkstation.o ls_uart.o
> +obj-$(CONFIG_HOLLY) += holly.o
> --- /dev/null
> +++ linux-2.6/arch/powerpc/platforms/embedded6xx/holly.c
> @@ -0,0 +1,326 @@
> +/*
> + * Board setup routines for the IBM 750GX/CL platform w/ TSI10x bridge
> + *
> + * Copyright 2007 IBM Corporation
> + *
> + * Stephen Winiecki <stevewin@us.ibm.com>
> + * Josh Boyer <jwboyer@linux.vnet.ibm.com>
> + *
> + * Based on code from mpc7448_hpc2.c
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + */
> +
> +#include <linux/stddef.h>
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +#include <linux/kdev_t.h>
> +#include <linux/console.h>
> +#include <linux/delay.h>
> +#include <linux/irq.h>
> +#include <linux/ide.h>
> +#include <linux/seq_file.h>
> +#include <linux/root_dev.h>
> +#include <linux/serial.h>
> +#include <linux/tty.h>
> +#include <linux/serial_core.h>
> +
> +#include <asm/system.h>
> +#include <asm/time.h>
> +#include <asm/machdep.h>
> +#include <asm/prom.h>
> +#include <asm/udbg.h>
> +#include <asm/tsi108.h>
> +#include <asm/pci-bridge.h>
> +#include <asm/reg.h>
> +#include <mm/mmu_decl.h>
> +#include "holly.h"
> +#include <asm/tsi108_irq.h>
> +#include <asm/mpic.h>
> +#include <asm/of_platform.h>
> +
> +#undef DEBUG
> +#ifdef DEBUG
> +#define DBG(fmt...) do { printk(fmt); } while(0)
> +#else
> +#define DBG(fmt...) do { } while(0)
> +#endif
Please use pr_debug() instead.
> +#ifndef CONFIG_PCI
> +pci_dram_offset = PPC750GXCL_TSI_PCI_MEM_OFFSET;
> +#endif
What's this about?
> +extern int tsi108_setup_pci(struct device_node *dev);
> +extern void tsi108_pci_int_init(struct device_node *node);
> +extern void tsi108_irq_cascade(unsigned int irq, struct irq_desc *desc);
> +
> +int holly_exclude_device(u_char bus, u_char devfn)
> +{
> + if (bus == 0 && PCI_SLOT(devfn) == 0)
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + else
> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static void holly_remap_bridge(void)
> +{
> + u32 lut_val, lut_addr = 0x900, misc_cfg;
Please initialize lut_addr right before using it instead of here, especially
since you re-assign it later on.
> + int i;
> +
> + printk(KERN_ERR "Remapping PCI bridge\n");
> +
> + /* Re-init the PCI bridge and LUT registers to have mappings that don't
> + * rely on PIBS */
Comment style
> + for (i = 0; i < 31; i++) {
> + tsi108_write_reg(TSI108_PB_OFFSET + lut_addr, 0x00000201);
> + lut_addr += 4;
> + tsi108_write_reg(TSI108_PB_OFFSET + lut_addr, 0x0);
> + lut_addr += 4;
> + }
> +
> + /* Reserve the last LUT entry for PCI I/O space */
> + tsi108_write_reg(TSI108_PB_OFFSET + lut_addr, 0x00000241);
> + lut_addr += 4;
> + tsi108_write_reg(TSI108_PB_OFFSET + lut_addr, 0x0);
> +
> + /* Map PCI I/O space */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x210, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x20c, 0x1);
> +
> + /* Map PCI CFG space */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x208, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x204, 0x7c000000 | 0x01);
> +
> + /* We don't need MEM32 and PRM remapping so disable them */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x214, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x220, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x230, 0x0);
It would be nice to get these constants defined up, but I know it's not always
practical. I'll bring it up anyway. :-)
> +
> + /* Set P2O_BAR0 */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x14, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x10, 0xc0000000);
> +
> + /* Init the PCI LUTs to do no remapping */
> + lut_addr = 0x500;
> + lut_val = 0x00000002;
> +
> + for (i = 0; i < 32; i++) {
> + tsi108_write_reg(TSI108_PCI_OFFSET + lut_addr, lut_val);
> + lut_addr += 4;
> + tsi108_write_reg(TSI108_PCI_OFFSET + lut_addr, 0x40000000);
> + lut_addr += 4;
> + lut_val += 0x02000000;
> + }
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x4c, 0x00007900);
> +
> + /* Set 64-bit PCI bus address for system memory */
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x1c, 0x0);
> + tsi108_write_reg(TSI108_PCI_OFFSET + 0x18, 0x0);
> +}
> +
> +static void __init holly_setup_arch(void)
> +{
> + struct device_node *cpu;
> + struct device_node *np;
> +
> + if (ppc_md.progress)
> + ppc_md.progress("holly_setup_arch():set_bridge", 0);
> +
> + cpu = of_find_node_by_type(NULL, "cpu");
> + if (cpu != 0) {
> + const unsigned int *fp;
> +
> + fp = of_get_property(cpu, "clock-frequency", NULL);
> + if (fp != 0)
> + loops_per_jiffy = *fp / HZ;
> + else
> + loops_per_jiffy = 50000000 / HZ;
> + of_node_put(cpu);
> + }
> + tsi108_csr_vir_base = get_vir_csrbase();
> +
> + /* setup PCI host bridge */
> + holly_remap_bridge();
> +#ifdef CONFIG_PCI
> + for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
> + tsi108_setup_pci(np);
On an environment without PCI, wouldn't it make more sense to leave it
out of the device tree, and thus not have a match in the above, instead
of taking out CONFIG_PCI?
> +
> + ppc_md.pci_exclude_device = holly_exclude_device;
> + if (ppc_md.progress)
> + ppc_md.progress("tsi108: resources set", 0x100);
> +#endif
> + printk(KERN_INFO "PPC750GX/CL Platform\n");
> +}
> +
> +/*
> + * Interrupt setup and service. Interrrupts on the holly come
> + * from the four external INT pins, PCI interrupts are routed via
> + * PCI interrupt control registers, it generates internal IRQ23
> + *
> + * Interrupt routing on the Holly Board:
> + * TSI108:PB_INT[0] -> CPU0:INT#
> + * TSI108:PB_INT[1] -> CPU0:MCP#
> + * TSI108:PB_INT[2] -> N/C
> + * TSI108:PB_INT[3] -> N/C
> + */
> +static void __init holly_init_IRQ(void)
> +{
> + struct mpic *mpic;
> + phys_addr_t mpic_paddr = 0;
> + struct device_node *tsi_pic;
> +#ifdef CONFIG_PCI
> + unsigned int cascade_pci_irq;
> + struct device_node *tsi_pci;
> + struct device_node *cascade_node = NULL;
> +#endif
> +
> + tsi_pic = of_find_node_by_type(NULL, "open-pic");
> + if (tsi_pic) {
> + unsigned int size;
> + const void *prop = of_get_property(tsi_pic, "reg", &size);
> + mpic_paddr = of_translate_address(tsi_pic, prop);
> + }
> +
> + if (mpic_paddr == 0) {
> + printk(KERN_ERR "%s: No tsi108 PIC found !\n", __func__);
> + return;
> + }
> +
> + DBG("%s: tsi108 pic phys_addr = 0x%x\n", __func__, (u32) mpic_paddr);
> +
> + mpic = mpic_alloc(tsi_pic, mpic_paddr,
> + MPIC_PRIMARY | MPIC_BIG_ENDIAN | MPIC_WANTS_RESET |
> + MPIC_SPV_EOI | MPIC_NO_PTHROU_DIS | MPIC_REGSET_TSI108,
> + 24,
> + NR_IRQS-4, /* num_sources used */
> + "Tsi108_PIC");
> +
> + BUG_ON(mpic == NULL);
> +
> + mpic_assign_isu(mpic, 0, mpic_paddr + 0x100);
> +
> + mpic_init(mpic);
> +
> +#ifdef CONFIG_PCI
> + tsi_pci = of_find_node_by_type(NULL, "pci");
> + if (tsi_pci == NULL) {
> + printk(KERN_ERR "%s: No tsi108 pci node found !\n", __func__);
> + return;
> + }
> +
> + cascade_node = of_find_node_by_type(NULL, "pic-router");
> + if (cascade_node == NULL) {
> + printk(KERN_ERR "%s: No tsi108 pci cascade node found !\n", __func__);
> + return;
> + }
> +
> + cascade_pci_irq = irq_of_parse_and_map(tsi_pci, 0);
> + DBG("%s: tsi108 cascade_pci_irq = 0x%x\n", __func__, (u32) cascade_pci_irq);
> + tsi108_pci_int_init(cascade_node);
> + set_irq_data(cascade_pci_irq, mpic);
> + set_irq_chained_handler(cascade_pci_irq, tsi108_irq_cascade);
> +#endif
> + /* Configure MPIC outputs to CPU0 */
> + tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0);
> + of_node_put(tsi_pic);
> +}
> +
> +void holly_show_cpuinfo(struct seq_file *m)
> +{
> + seq_printf(m, "vendor\t\t: IBM\n");
> + seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
> +}
> +
> +void holly_restart(char *cmd)
> +{
> + unsigned long *ocn_bar1 = NULL;
> + unsigned long bar;
> + struct device_node *bridge = NULL;
> + const void *prop;
> + int size;
> + phys_addr_t addr = 0xc0000000;
> +
> + local_irq_disable();
> +
> + bridge = of_find_node_by_type(NULL, "tsi-bridge");
> + if (bridge) {
> + prop = of_get_property(bridge, "reg", &size);
> + addr = of_translate_address(bridge, prop);
> + }
> + addr += (TSI108_PB_OFFSET + 0x414);
> +
> + ocn_bar1 = ioremap(addr, 0x4);
> +
> + /* Turn on the BOOT bit so the addresses are correctly
> + * routed to the HLP interface */
> + bar = ioread32be(ocn_bar1);
> + bar |= 2;
> + iowrite32be(bar, ocn_bar1);
> + iosync();
> +
> + /* Set SRR0 to the reset vector and turn on MSR_IP */
> + mtspr(SPRN_SRR0, 0xfff00100);
> + mtspr(SPRN_SRR1, MSR_IP);
> +
> + /* Do an rfi to jump back to firmware. Somewhat evil,
> + * but it works
> + */
> + __asm__ __volatile__("rfi" : : : "memory");
Evil, yes, but it should work. Actually, it should work for me as well,
thanks for the tip. :)
> +
> + /* Spin until reset happens. Shouldn't really get here */
> + for (;;) ;
> +}
> +
> +void holly_power_off(void)
> +{
> + local_irq_disable();
> + /* No way to shut power off with software */
> + for (;;) ;
> +}
> +
> +void holly_halt(void)
> +{
> + holly_power_off();
> +}
> +
> +/*
> + * Called very early, device-tree isn't unflattened
> + */
> +static int __init holly_probe(void)
> +{
> + unsigned long root = of_get_flat_dt_root();
> +
> + if (!of_flat_dt_is_compatible(root, "ppc750"))
> + return 0;
That's a pretty broad compatible check?
> + return 1;
> +}
> +
> +static int ppc750_machine_check_exception(struct pt_regs *regs)
> +{
> + extern void tsi108_clear_pci_cfg_error(void);
> + const struct exception_table_entry *entry;
> +
> + /* Are we prepared to handle this fault */
> + if ((entry = search_exception_tables(regs->nip)) != NULL) {
> + tsi108_clear_pci_cfg_error();
> + regs->msr |= MSR_RI;
> + regs->nip = entry->fixup;
> + return 1;
> + }
> + return 0;
> +}
> +
> +define_machine(holly){
> + .name = "PPC750 GX/CL TSI",
> + .probe = holly_probe,
> + .setup_arch = holly_setup_arch,
> + .init_IRQ = holly_init_IRQ,
> + .show_cpuinfo = holly_show_cpuinfo,
> + .get_irq = mpic_get_irq,
> + .restart = holly_restart,
> + .calibrate_decr = generic_calibrate_decr,
> + .machine_check_exception = ppc750_machine_check_exception,
> + .progress = udbg_progress,
These look all unaligned for me.
> +};
> --- /dev/null
> +++ linux-2.6/arch/powerpc/platforms/embedded6xx/holly.h
> @@ -0,0 +1,24 @@
> +/*
> + * Definitions for the IBM 750GX/CL platform w/ TSI10x bridge
> + *
> + * Copyright 2007 IBM Corporation
> + *
> + * Stephen Winiecki <stevewin@us.ibm.com>
> + * Josh Boyer <jwboyer@linux.vnet.ibm.com>
> + *
> + * Based on code from mpc7448_hpc2.h
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + */
> +
> +#ifndef __PPC_PLATFORMS_PPC750GXCL_TSI_H
> +#define __PPC_PLATFORMS_PPC750GXCL_TSI_H
> +
> +#include <asm/ppcboot.h>
> +
> +/* Base Addresses for the PCI bus
> + */
> +#define PPC750GXCL_TSI_PCI_MEM_OFFSET (0x00000000)
> +#endif /* __PPC_PLATFORMS_H */
> --- linux-2.6.orig/drivers/net/tsi108_eth.h
> +++ linux-2.6/drivers/net/tsi108_eth.h
> @@ -49,7 +49,11 @@
> */
> #define PHY_MV88E 1 /* Marvel 88Exxxx PHY */
> #define PHY_BCM54XX 2 /* Broardcom BCM54xx PHY */
> +#if defined(CONFIG_HOLLY)
> +#define TSI108_PHY_TYPE PHY_BCM54XX
> +#else
> #define TSI108_PHY_TYPE PHY_MV88E
> +#endif
This won't work well if you ever want to build a multiplatform kernel.
>
> /*
> * TSI108 GIGE port registers
> --- linux-2.6.orig/include/asm-powerpc/tsi108.h
> +++ linux-2.6/include/asm-powerpc/tsi108.h
> @@ -68,7 +68,11 @@
> #define TSI108_PB_ERRCS_ES (1 << 1)
> #define TSI108_PB_ISR_PBS_RD_ERR (1 << 8)
>
> +#ifdef CONFIG_HOLLY
> +#define TSI108_PCI_CFG_BASE_PHYS (0x7c000000)
> +#else
> #define TSI108_PCI_CFG_BASE_PHYS (0xfb000000)
> +#endif
> #define TSI108_PCI_CFG_SIZE (0x01000000)
Same here. Shouldn't this come out of the devicetree?
-Olof
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 19:01 ` Olof Johansson
@ 2007-05-04 19:18 ` Josh Boyer
2007-05-08 23:17 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 25+ messages in thread
From: Josh Boyer @ 2007-05-04 19:18 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev
On Fri, 2007-05-04 at 14:01 -0500, Olof Johansson wrote:
> > +#undef DEBUG
> > +#ifdef DEBUG
> > +#define DBG(fmt...) do { printk(fmt); } while(0)
> > +#else
> > +#define DBG(fmt...) do { } while(0)
> > +#endif
>
> Please use pr_debug() instead.
/me hides head in shame. I'm guilt of the copy/paste crime. Will fix.
> > +#ifndef CONFIG_PCI
> > +pci_dram_offset = PPC750GXCL_TSI_PCI_MEM_OFFSET;
> > +#endif
>
> What's this about?
To be honest, I'm not sure. I haven't tried compiling without PCI.
Just following the Taiga board's example again.
> > +static void holly_remap_bridge(void)
> > +{
> > + u32 lut_val, lut_addr = 0x900, misc_cfg;
>
> Please initialize lut_addr right before using it instead of here, especially
> since you re-assign it later on.
Ok.
>
> > + int i;
> > +
> > + printk(KERN_ERR "Remapping PCI bridge\n");
> > +
> > + /* Re-init the PCI bridge and LUT registers to have mappings that don't
> > + * rely on PIBS */
>
> Comment style
Yep.
> > +
> > + /* We don't need MEM32 and PRM remapping so disable them */
> > + tsi108_write_reg(TSI108_PCI_OFFSET + 0x214, 0x0);
> > + tsi108_write_reg(TSI108_PCI_OFFSET + 0x220, 0x0);
> > + tsi108_write_reg(TSI108_PCI_OFFSET + 0x230, 0x0);
>
> It would be nice to get these constants defined up, but I know it's not always
> practical. I'll bring it up anyway. :-)
This is part of the on-going PCI fixup stuff. It'll get cleaned up in
the final version.
> > + /* setup PCI host bridge */
> > + holly_remap_bridge();
> > +#ifdef CONFIG_PCI
> > + for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
> > + tsi108_setup_pci(np);
>
> On an environment without PCI, wouldn't it make more sense to leave it
> out of the device tree, and thus not have a match in the above, instead
> of taking out CONFIG_PCI?
Yes, actually. Good catch.
> > +static int __init holly_probe(void)
> > +{
> > + unsigned long root = of_get_flat_dt_root();
> > +
> > + if (!of_flat_dt_is_compatible(root, "ppc750"))
> > + return 0;
>
> That's a pretty broad compatible check?
Yeah, will fix.
> > +define_machine(holly){
> > + .name = "PPC750 GX/CL TSI",
> > + .probe = holly_probe,
> > + .setup_arch = holly_setup_arch,
> > + .init_IRQ = holly_init_IRQ,
> > + .show_cpuinfo = holly_show_cpuinfo,
> > + .get_irq = mpic_get_irq,
> > + .restart = holly_restart,
> > + .calibrate_decr = generic_calibrate_decr,
> > + .machine_check_exception = ppc750_machine_check_exception,
> > + .progress = udbg_progress,
>
> These look all unaligned for me.
Erg.
> > --- linux-2.6.orig/drivers/net/tsi108_eth.h
> > +++ linux-2.6/drivers/net/tsi108_eth.h
> > @@ -49,7 +49,11 @@
> > */
> > #define PHY_MV88E 1 /* Marvel 88Exxxx PHY */
> > #define PHY_BCM54XX 2 /* Broardcom BCM54xx PHY */
> > +#if defined(CONFIG_HOLLY)
> > +#define TSI108_PHY_TYPE PHY_BCM54XX
> > +#else
> > #define TSI108_PHY_TYPE PHY_MV88E
> > +#endif
>
> This won't work well if you ever want to build a multiplatform kernel.
I know. What do you suggest? TSI doesn't use phylib, and adding a
property to specify PHY type in the DT seems like a hack too...
> > * TSI108 GIGE port registers
> > --- linux-2.6.orig/include/asm-powerpc/tsi108.h
> > +++ linux-2.6/include/asm-powerpc/tsi108.h
> > @@ -68,7 +68,11 @@
> > #define TSI108_PB_ERRCS_ES (1 << 1)
> > #define TSI108_PB_ISR_PBS_RD_ERR (1 << 8)
> >
> > +#ifdef CONFIG_HOLLY
> > +#define TSI108_PCI_CFG_BASE_PHYS (0x7c000000)
> > +#else
> > #define TSI108_PCI_CFG_BASE_PHYS (0xfb000000)
> > +#endif
> > #define TSI108_PCI_CFG_SIZE (0x01000000)
>
> Same here. Shouldn't this come out of the devicetree?
As a "reg" property perhaps? I'm not a DT guru, so I was a bit lost as
to how to fix that up.
josh
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 19:18 ` Josh Boyer
@ 2007-05-08 23:17 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 25+ messages in thread
From: Benjamin Herrenschmidt @ 2007-05-08 23:17 UTC (permalink / raw)
To: Josh Boyer; +Cc: Olof Johansson, linuxppc-dev
> > This won't work well if you ever want to build a multiplatform kernel.
>
> I know. What do you suggest? TSI doesn't use phylib, and adding a
> property to specify PHY type in the DT seems like a hack too...
Fix the TSI network driver. PHYs can be probed.
> As a "reg" property perhaps? I'm not a DT guru, so I was a bit lost as
> to how to fix that up.
Yes, it's common to get the config space address of the "reg" property
of the bridge.
Ben.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 18:14 ` [PATCH 1/4] " Josh Boyer
2007-05-04 18:35 ` Kumar Gala
2007-05-04 19:01 ` Olof Johansson
@ 2007-05-04 19:44 ` Arnd Bergmann
2007-05-04 20:01 ` Josh Boyer
` (2 more replies)
2 siblings, 3 replies; 25+ messages in thread
From: Arnd Bergmann @ 2007-05-04 19:44 UTC (permalink / raw)
To: linuxppc-dev
On Friday 04 May 2007, Josh Boyer wrote:
> +
> +#undef DEBUG
> +#ifdef DEBUG
> +#define DBG(fmt...) do { printk(fmt); } while(0)
> +#else
> +#define DBG(fmt...) do { } while(0)
> +#endif
Please replace this with the generic pr_debug()
> +#ifndef CONFIG_PCI
> +pci_dram_offset = PPC750GXCL_TSI_PCI_MEM_OFFSET;
> +#endif
This looks like the compile would expect a type.
> +extern int tsi108_setup_pci(struct device_node *dev);
> +extern void tsi108_pci_int_init(struct device_node *node);
> +extern void tsi108_irq_cascade(unsigned int irq, struct irq_desc *desc);
please move any extern declarations into a header file that
is included by both the file that defines and uses them
> +static void holly_remap_bridge(void)
> +{
> + u32 lut_val, lut_addr = 0x900, misc_cfg;
> + int i;
> +
> + printk(KERN_ERR "Remapping PCI bridge\n");
Is it an error to remap the bridge? If not, KERN_INFO would be more
appropriate ;-)
> +void holly_show_cpuinfo(struct seq_file *m)
> +{
> + seq_printf(m, "vendor\t\t: IBM\n");
> + seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
> +}
If it's an IBM product, it should come with a product code like 123-4567,
which fits in here, instead of just listing the CPU.
> +static int ppc750_machine_check_exception(struct pt_regs *regs)
> +{
> + extern void tsi108_clear_pci_cfg_error(void);
move declaration to header file.
> + const struct exception_table_entry *entry;
> +
> + /* Are we prepared to handle this fault */
> + if ((entry = search_exception_tables(regs->nip)) != NULL) {
> + tsi108_clear_pci_cfg_error();
> + regs->msr |= MSR_RI;
> + regs->nip = entry->fixup;
> + return 1;
> + }
> + return 0;
> +}
Are you sure that you can use the generic exception table mechanism
like this? I can't see why it doesn't work, but it's something I haven't
seen anyone do like this.
> --- linux-2.6.orig/drivers/net/tsi108_eth.h
> +++ linux-2.6/drivers/net/tsi108_eth.h
> @@ -49,7 +49,11 @@
> */
> #define PHY_MV88E 1 /* Marvel 88Exxxx PHY */
> #define PHY_BCM54XX 2 /* Broardcom BCM54xx PHY */
> +#if defined(CONFIG_HOLLY)
> +#define TSI108_PHY_TYPE PHY_BCM54XX
> +#else
> #define TSI108_PHY_TYPE PHY_MV88E
> +#endif
>
this breaks multiplatform setups.
> --- linux-2.6.orig/include/asm-powerpc/tsi108.h
> +++ linux-2.6/include/asm-powerpc/tsi108.h
> @@ -68,7 +68,11 @@
> #define TSI108_PB_ERRCS_ES (1 << 1)
> #define TSI108_PB_ISR_PBS_RD_ERR (1 << 8)
>
> +#ifdef CONFIG_HOLLY
> +#define TSI108_PCI_CFG_BASE_PHYS (0x7c000000)
> +#else
> #define TSI108_PCI_CFG_BASE_PHYS (0xfb000000)
> +#endif
> #define TSI108_PCI_CFG_SIZE (0x01000000)
> /* Global variables */
>
Same here.
Arnd <><
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 19:44 ` Arnd Bergmann
@ 2007-05-04 20:01 ` Josh Boyer
2007-05-05 11:08 ` Arnd Bergmann
2007-05-08 23:20 ` Benjamin Herrenschmidt
2007-05-05 14:39 ` Olof Johansson
2007-05-08 23:19 ` Benjamin Herrenschmidt
2 siblings, 2 replies; 25+ messages in thread
From: Josh Boyer @ 2007-05-04 20:01 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
On Fri, 2007-05-04 at 21:44 +0200, Arnd Bergmann wrote:
> On Friday 04 May 2007, Josh Boyer wrote:
> > +
> > +#undef DEBUG
> > +#ifdef DEBUG
> > +#define DBG(fmt...) do { printk(fmt); } while(0)
> > +#else
> > +#define DBG(fmt...) do { } while(0)
> > +#endif
>
> Please replace this with the generic pr_debug()
Already noted, but yes.
> > +extern int tsi108_setup_pci(struct device_node *dev);
> > +extern void tsi108_pci_int_init(struct device_node *node);
> > +extern void tsi108_irq_cascade(unsigned int irq, struct irq_desc *desc);
>
> please move any extern declarations into a header file that
> is included by both the file that defines and uses them
Ok.
>
> > +static void holly_remap_bridge(void)
> > +{
> > + u32 lut_val, lut_addr = 0x900, misc_cfg;
> > + int i;
> > +
> > + printk(KERN_ERR "Remapping PCI bridge\n");
>
> Is it an error to remap the bridge? If not, KERN_INFO would be more
> appropriate ;-)
This whole function is under work at the moment, so it'll get cleaned
up.
>
> > +void holly_show_cpuinfo(struct seq_file *m)
> > +{
> > + seq_printf(m, "vendor\t\t: IBM\n");
> > + seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
> > +}
>
> If it's an IBM product, it should come with a product code like 123-4567,
> which fits in here, instead of just listing the CPU.
Erm... why? There are other boards that don't do this as well...
>
> > +static int ppc750_machine_check_exception(struct pt_regs *regs)
> > +{
> > + extern void tsi108_clear_pci_cfg_error(void);
>
> move declaration to header file.
Ok.
>
> > + const struct exception_table_entry *entry;
> > +
> > + /* Are we prepared to handle this fault */
> > + if ((entry = search_exception_tables(regs->nip)) != NULL) {
> > + tsi108_clear_pci_cfg_error();
> > + regs->msr |= MSR_RI;
> > + regs->nip = entry->fixup;
> > + return 1;
> > + }
> > + return 0;
> > +}
>
> Are you sure that you can use the generic exception table mechanism
> like this? I can't see why it doesn't work, but it's something I haven't
> seen anyone do like this.
Yes, this works and is actually required. See tsi108_pci.c for where
the fixup/exception table information is generated.
> > --- linux-2.6.orig/drivers/net/tsi108_eth.h
> > +++ linux-2.6/drivers/net/tsi108_eth.h
> > @@ -49,7 +49,11 @@
> > */
> > #define PHY_MV88E 1 /* Marvel 88Exxxx PHY */
> > #define PHY_BCM54XX 2 /* Broardcom BCM54xx PHY */
> > +#if defined(CONFIG_HOLLY)
> > +#define TSI108_PHY_TYPE PHY_BCM54XX
> > +#else
> > #define TSI108_PHY_TYPE PHY_MV88E
> > +#endif
> >
>
> this breaks multiplatform setups.
I know. Looking for a better suggestions. Kumar suggested a Kconfig
option at least, which is what I'm going to go with unless someone has a
better idea...
josh
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 20:01 ` Josh Boyer
@ 2007-05-05 11:08 ` Arnd Bergmann
2007-05-05 11:44 ` Josh Boyer
` (2 more replies)
2007-05-08 23:20 ` Benjamin Herrenschmidt
1 sibling, 3 replies; 25+ messages in thread
From: Arnd Bergmann @ 2007-05-05 11:08 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
On Friday 04 May 2007, Josh Boyer wrote:
>=20
> > If it's an IBM product, it should come with a product code like 123-456=
7,
> > which fits in here, instead of just listing the CPU.
>=20
> Erm... why? =A0There are other boards that don't do this as well...
We did it on the Cell blade because we were asked to do it by
the people responsible for the hardware, who were following
their rules. I assumed that the same rules apply here as well,
but maybe they are specific to the server line.
> > > --- linux-2.6.orig/drivers/net/tsi108_eth.h
> > > +++ linux-2.6/drivers/net/tsi108_eth.h
> > > @@ -49,7 +49,11 @@
> > > =A0 */
> > > =A0#define PHY_MV88E=A0=A01=A0=A0=A0=A0=A0=A0=A0/* Marvel 88Exxxx PHY=
*/
> > > =A0#define PHY_BCM54XX=A0=A0=A0=A0=A0=A0=A0=A02=A0=A0=A0=A0=A0=A0=A0/=
* Broardcom BCM54xx PHY */
> > > +#if defined(CONFIG_HOLLY)
> > > +#define TSI108_PHY_TYPE PHY_BCM54XX
> > > +#else
> > > =A0#define TSI108_PHY_TYPE=A0=A0=A0=A0PHY_MV88E
> > > +#endif
> > > =A0
> >=20
> > this breaks multiplatform setups.
>=20
> I know. =A0Looking for a better suggestions. =A0Kumar suggested a Kconfig
> option at least, which is what I'm going to go with unless someone has a
> better idea...
A Kconfig option won't help at all. The phy should come as a device node
below the ethernet device. When probing the device, look at the
"compatible" and "reg" properties in there.
Arnd <><
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-05 11:08 ` Arnd Bergmann
@ 2007-05-05 11:44 ` Josh Boyer
2007-05-05 14:41 ` Olof Johansson
2007-05-08 23:24 ` Benjamin Herrenschmidt
2 siblings, 0 replies; 25+ messages in thread
From: Josh Boyer @ 2007-05-05 11:44 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
On Sat, 2007-05-05 at 13:08 +0200, Arnd Bergmann wrote:
> On Friday 04 May 2007, Josh Boyer wrote:
> >
> > > If it's an IBM product, it should come with a product code like 123-4567,
> > > which fits in here, instead of just listing the CPU.
> >
> > Erm... why? There are other boards that don't do this as well...
>
> We did it on the Cell blade because we were asked to do it by
> the people responsible for the hardware, who were following
> their rules. I assumed that the same rules apply here as well,
> but maybe they are specific to the server line.
I've heard of no such rule from the people I've been working with. This
is an internal matter though, so I'll do some checking.
> > > > --- linux-2.6.orig/drivers/net/tsi108_eth.h
> > > > +++ linux-2.6/drivers/net/tsi108_eth.h
> > > > @@ -49,7 +49,11 @@
> > > > */
> > > > #define PHY_MV88E 1 /* Marvel 88Exxxx PHY */
> > > > #define PHY_BCM54XX 2 /* Broardcom BCM54xx PHY */
> > > > +#if defined(CONFIG_HOLLY)
> > > > +#define TSI108_PHY_TYPE PHY_BCM54XX
> > > > +#else
> > > > #define TSI108_PHY_TYPE PHY_MV88E
> > > > +#endif
> > > >
> > >
> > > this breaks multiplatform setups.
> >
> > I know. Looking for a better suggestions. Kumar suggested a Kconfig
> > option at least, which is what I'm going to go with unless someone has a
> > better idea...
>
> A Kconfig option won't help at all. The phy should come as a device node
> below the ethernet device. When probing the device, look at the
> "compatible" and "reg" properties in there.
A Kconfig option is better than what's currently there, but yes I know
what you mean. The phys are already part of the device tree, so I think
I'll add a compatible property as you suggested and add that to the
hw_info structure that the ethernet driver uses to init things.
Seems silly to carry this around in a data structure for a single use,
but I don't see another way of doing it while still making the
multiplatform thing happy.
josh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-05 11:08 ` Arnd Bergmann
2007-05-05 11:44 ` Josh Boyer
@ 2007-05-05 14:41 ` Olof Johansson
2007-05-08 23:24 ` Benjamin Herrenschmidt
2 siblings, 0 replies; 25+ messages in thread
From: Olof Johansson @ 2007-05-05 14:41 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
On Sat, May 05, 2007 at 01:08:11PM +0200, Arnd Bergmann wrote:
> On Friday 04 May 2007, Josh Boyer wrote:
> >
> > > If it's an IBM product, it should come with a product code like 123-4567,
> > > which fits in here, instead of just listing the CPU.
> >
> > Erm... why? ?There are other boards that don't do this as well...
>
> We did it on the Cell blade because we were asked to do it by
> the people responsible for the hardware, who were following
> their rules. I assumed that the same rules apply here as well,
> but maybe they are specific to the server line.
>
> > > > --- linux-2.6.orig/drivers/net/tsi108_eth.h
> > > > +++ linux-2.6/drivers/net/tsi108_eth.h
> > > > @@ -49,7 +49,11 @@
> > > > ? */
> > > > ?#define PHY_MV88E??1???????/* Marvel 88Exxxx PHY */
> > > > ?#define PHY_BCM54XX????????2???????/* Broardcom BCM54xx PHY */
> > > > +#if defined(CONFIG_HOLLY)
> > > > +#define TSI108_PHY_TYPE PHY_BCM54XX
> > > > +#else
> > > > ?#define TSI108_PHY_TYPE????PHY_MV88E
> > > > +#endif
> > > > ?
> > >
> > > this breaks multiplatform setups.
> >
> > I know. ?Looking for a better suggestions. ?Kumar suggested a Kconfig
> > option at least, which is what I'm going to go with unless someone has a
> > better idea...
>
> A Kconfig option won't help at all. The phy should come as a device node
> below the ethernet device. When probing the device, look at the
> "compatible" and "reg" properties in there.
In a perfect world, yes, but does it need to hold up the rest of the
code being merged if it's changed later? It's just used to blast a few
values at init time now, I'm suspecting to set things such as LED settings
(but I can't verify since I don't have any broadcom datasheets).
Even better long-term would be to have a phylib driver for the TSI and
autoprobe the model. Neither should have to hold up a board port merge
in my opinion.
-Olof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-05 11:08 ` Arnd Bergmann
2007-05-05 11:44 ` Josh Boyer
2007-05-05 14:41 ` Olof Johansson
@ 2007-05-08 23:24 ` Benjamin Herrenschmidt
2 siblings, 0 replies; 25+ messages in thread
From: Benjamin Herrenschmidt @ 2007-05-08 23:24 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
> A Kconfig option won't help at all. The phy should come as a device node
> below the ethernet device. When probing the device, look at the
> "compatible" and "reg" properties in there.
That's overkill. You don't need that unless you add specific
config/setup informations for the PHY. PHYs can be probed easily enough.
Ben.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 20:01 ` Josh Boyer
2007-05-05 11:08 ` Arnd Bergmann
@ 2007-05-08 23:20 ` Benjamin Herrenschmidt
2007-05-09 0:32 ` Josh Boyer
1 sibling, 1 reply; 25+ messages in thread
From: Benjamin Herrenschmidt @ 2007-05-08 23:20 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, Arnd Bergmann
> I know. Looking for a better suggestions. Kumar suggested a Kconfig
> option at least, which is what I'm going to go with unless someone has a
> better idea...
Yes, I do: Fix the TSI driver, it should be trivial :-)
Ben.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 19:44 ` Arnd Bergmann
2007-05-04 20:01 ` Josh Boyer
@ 2007-05-05 14:39 ` Olof Johansson
2007-05-05 15:11 ` Josh Boyer
2007-05-08 23:19 ` Benjamin Herrenschmidt
2 siblings, 1 reply; 25+ messages in thread
From: Olof Johansson @ 2007-05-05 14:39 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
On Fri, May 04, 2007 at 09:44:30PM +0200, Arnd Bergmann wrote:
> > --- linux-2.6.orig/include/asm-powerpc/tsi108.h
> > +++ linux-2.6/include/asm-powerpc/tsi108.h
> > @@ -68,7 +68,11 @@
> > #define TSI108_PB_ERRCS_ES (1 << 1)
> > #define TSI108_PB_ISR_PBS_RD_ERR (1 << 8)
> >
> > +#ifdef CONFIG_HOLLY
> > +#define TSI108_PCI_CFG_BASE_PHYS (0x7c000000)
> > +#else
> > #define TSI108_PCI_CFG_BASE_PHYS (0xfb000000)
> > +#endif
> > #define TSI108_PCI_CFG_SIZE (0x01000000)
> > /* Global variables */
> >
>
> Same here.
It would be very easy to take tsi108_setup_pci take the config base as
a function paramter and pass it in from the board setup code, and store
it in a static variable to be used for the tsi108_clear_pci_cfg_error
function.
That'd easily make the code multiplatform friendly without having to
add it to the device tree. Very few other platforms have the config base
address in there, if any. It shouldn't be a requirement that Holly has it.
-Olof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-05 14:39 ` Olof Johansson
@ 2007-05-05 15:11 ` Josh Boyer
2007-05-06 0:09 ` Arnd Bergmann
0 siblings, 1 reply; 25+ messages in thread
From: Josh Boyer @ 2007-05-05 15:11 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev, Arnd Bergmann
On Sat, 2007-05-05 at 09:39 -0500, Olof Johansson wrote:
> On Fri, May 04, 2007 at 09:44:30PM +0200, Arnd Bergmann wrote:
>
> > > --- linux-2.6.orig/include/asm-powerpc/tsi108.h
> > > +++ linux-2.6/include/asm-powerpc/tsi108.h
> > > @@ -68,7 +68,11 @@
> > > #define TSI108_PB_ERRCS_ES (1 << 1)
> > > #define TSI108_PB_ISR_PBS_RD_ERR (1 << 8)
> > >
> > > +#ifdef CONFIG_HOLLY
> > > +#define TSI108_PCI_CFG_BASE_PHYS (0x7c000000)
> > > +#else
> > > #define TSI108_PCI_CFG_BASE_PHYS (0xfb000000)
> > > +#endif
> > > #define TSI108_PCI_CFG_SIZE (0x01000000)
> > > /* Global variables */
> > >
> >
> > Same here.
>
> It would be very easy to take tsi108_setup_pci take the config base as
> a function paramter and pass it in from the board setup code, and store
> it in a static variable to be used for the tsi108_clear_pci_cfg_error
> function.
>
> That'd easily make the code multiplatform friendly without having to
> add it to the device tree. Very few other platforms have the config base
> address in there, if any. It shouldn't be a requirement that Holly has it.
Yeah, after thinking about it, that's what I plan on doing actually.
josh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-05 15:11 ` Josh Boyer
@ 2007-05-06 0:09 ` Arnd Bergmann
0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2007-05-06 0:09 UTC (permalink / raw)
To: Josh Boyer; +Cc: Olof Johansson, linuxppc-dev
On Saturday 05 May 2007, Josh Boyer wrote:
> >
> > It would be very easy to take tsi108_setup_pci take the config base as
> > a function paramter and pass it in from the board setup code, and store
> > it in a static variable to be used for the tsi108_clear_pci_cfg_error
> > function.
> >
> > That'd easily make the code multiplatform friendly without having to
> > add it to the device tree. Very few other platforms have the config base
> > address in there, if any. It shouldn't be a requirement that Holly has it.
>
> Yeah, after thinking about it, that's what I plan on doing actually.
>
Yes, sounds fair enough. The more interesting bits to clean up in the
driver are to convert it to an of_platform_driver and to use phylib, and
then autodetecting this will be trivial, and until then hardcoding in the
board setup seems a good solution.
Arnd <><
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-04 19:44 ` Arnd Bergmann
2007-05-04 20:01 ` Josh Boyer
2007-05-05 14:39 ` Olof Johansson
@ 2007-05-08 23:19 ` Benjamin Herrenschmidt
2007-05-09 0:39 ` Josh Boyer
2 siblings, 1 reply; 25+ messages in thread
From: Benjamin Herrenschmidt @ 2007-05-08 23:19 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
On Fri, 2007-05-04 at 21:44 +0200, Arnd Bergmann wrote:
> > +void holly_show_cpuinfo(struct seq_file *m)
> > +{
> > + seq_printf(m, "vendor\t\t: IBM\n");
> > + seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
> > +}
>
> If it's an IBM product, it should come with a product code like 123-4567,
> which fits in here, instead of just listing the CPU.
Should probably come from the device tree anyway.
> > +static int ppc750_machine_check_exception(struct pt_regs *regs)
> > +{
> > + extern void tsi108_clear_pci_cfg_error(void);
>
> move declaration to header file.
>
> > + const struct exception_table_entry *entry;
> > +
> > + /* Are we prepared to handle this fault */
> > + if ((entry = search_exception_tables(regs->nip)) != NULL) {
> > + tsi108_clear_pci_cfg_error();
> > + regs->msr |= MSR_RI;
> > + regs->nip = entry->fixup;
> > + return 1;
> > + }
> > + return 0;
> > +}
>
> Are you sure that you can use the generic exception table mechanism
> like this? I can't see why it doesn't work, but it's something I haven't
> seen anyone do like this.
Also, can't the TSI be configured to not generate MCE in that case ?
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/4] Add support for 750CL Holly board
2007-05-08 23:19 ` Benjamin Herrenschmidt
@ 2007-05-09 0:39 ` Josh Boyer
0 siblings, 0 replies; 25+ messages in thread
From: Josh Boyer @ 2007-05-09 0:39 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Arnd Bergmann
On Wed, 2007-05-09 at 09:19 +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2007-05-04 at 21:44 +0200, Arnd Bergmann wrote:
>
> > > +void holly_show_cpuinfo(struct seq_file *m)
> > > +{
> > > + seq_printf(m, "vendor\t\t: IBM\n");
> > > + seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
> > > +}
> >
> > If it's an IBM product, it should come with a product code like 123-4567,
> > which fits in here, instead of just listing the CPU.
>
> Should probably come from the device tree anyway.
It will with some cleanup patches I'm planning on.
> > > + const struct exception_table_entry *entry;
> > > +
> > > + /* Are we prepared to handle this fault */
> > > + if ((entry = search_exception_tables(regs->nip)) != NULL) {
> > > + tsi108_clear_pci_cfg_error();
> > > + regs->msr |= MSR_RI;
> > > + regs->nip = entry->fixup;
> > > + return 1;
> > > + }
> > > + return 0;
> > > +}
> >
> > Are you sure that you can use the generic exception table mechanism
> > like this? I can't see why it doesn't work, but it's something I haven't
> > seen anyone do like this.
>
> Also, can't the TSI be configured to not generate MCE in that case ?
Not that I'm aware of. They should only occur on the initial PCI bus
scan.
josh
^ permalink raw reply [flat|nested] 25+ messages in thread