Thread (23 messages) flat view 23 messages, 9 authors, 2007-03-08

Re: [patch 2/7] xenon: add platform support

From: Arnd Bergmann <arnd@arndb.de>
Date: 2007-03-07 23:29:34

+#define PRIO_IPI_4       0x08
+#define PRIO_IPI_3       0x10
+#define PRIO_SMM         0x14
+#define PRIO_SFCX        0x18
+#define PRIO_SATA_HDD    0x20
+#define PRIO_SATA_CDROM  0x24
+#define PRIO_OHCI_0      0x2c
+#define PRIO_EHCI_0      0x30
+#define PRIO_OHCI_1      0x34
+#define PRIO_EHCI_1      0x38
+#define PRIO_XMA         0x40
+#define PRIO_AUDIO       0x44
+#define PRIO_ENET        0x4C
+#define PRIO_XPS         0x54
+#define PRIO_GRAPHICS    0x58
+#define PRIO_PROFILER    0x60
+#define PRIO_BIU         0x64
+#define PRIO_IOC         0x68
+#define PRIO_FSB         0x6c
+#define PRIO_IPI_2       0x70
+#define PRIO_CLOCK       0x74
+#define PRIO_IPI_1       0x78
These should probably all come from the device tree
instead of compile-time constants.
+void __init xenon_iic_init_IRQ(void)
+{
+	int i;
+	struct device_node *dn;
+
+	printk("XENON init IRQ\n");
needs a printk level, KERN_DEBUG or KERN_INFO.
+			/* search for our interrupt controller inside the device tree */
+	for (dn = NULL;
+	     (dn = of_find_node_by_name(dn,"interrupt-controller")) != NULL;) {
+		if (!device_is_compatible(dn,
+				     "xenon"))
+			continue;
+
+		irq_set_virq_count(0x80);
+		iic_base = ioremap_nocache(0x20000050000, 0x10000);
This should come from the 'reg' property of the above node.

+void xenon_cause_IPI(int target, int msg)
+{
+	int ipi_prio;
+
+	ipi_prio = ipi_to_prio(msg);
+
+	__raw_writeq( (0x10000<<target) | ipi_prio, iic_base + 0x10 +
hard_smp_processor_id() * 0x1000); +}
+
You seem to have a line wrapping problem and might need to fix your
mail client.
Also, don't use __raw_writeq here. In order to access on-chip data,
use out_be64().
+	hose->ops = &xenon_pci_ops;
+	hose->cfg_addr = ioremap(0xd0000000, 0x1000000);
+
The addresses should come from the device tree, don't hardcode
them in the source.
+#define DEBUG
+
really?

+void __init xenon_pci_init(void);
+#ifdef CONFIG_SMP
+extern void smp_init_xenon(void);
+#endif
move these declarations to a header
+static void xenon_show_cpuinfo(struct seq_file *m)
+{
+	struct device_node *root;
+	const char *model = "";
+
+	root = of_find_node_by_path("/");
+	if (root)
+		model = get_property(root, "model", NULL);
+	seq_printf(m, "machine\t\t: CHRP %s\n", model);
+	of_node_put(root);
+}
CHRP???
+static void __init xenon_pcibios_fixup(void)
+{
+	struct pci_dev *dev = NULL;
+
+	for_each_pci_dev(dev)
+		pci_read_irq_line(dev);
+}
I guess you copied that from cell? You should not need it.
+	if (ROOT_DEV == 0) {
+		printk("No ramdisk, default root is /dev/hda2\n");
+		ROOT_DEV = Root_HDA2;
+	}
kill this, it's unnecessary (and probably wrong in your case)
+static int __init xenon_probe(void)
+{
+       hpte_init_native();
+
+       return 1;
+}
This definitely needs to check the device tree whether it's running
on the right platform, otherwise you can not build a multiplatform
kernel.
+void __init xenon_hpte_init(unsigned long htab_size);
+
move this to a header file

+#include <asm/rtas.h>
rtas???
+#ifdef DEBUG
+#define DBG(fmt...) printk(fmt)
+#else
+#define DBG(fmt...)
+#endif
+
Don't define your own macros like this, use the pr_debug()
one from kernel.h.
+void smp_init_xenon(void);
+
+extern void xenon_request_IPIs(void);
+extern void xenon_init_irq_on_cpu(int cpu);
+
+extern void xenon_cause_IPI(int target, int msg);
extern declarations should go to header files, not into
the implementation.

+	/* Mark threads which are still spinning in hold loops. */
+	if (cpu_has_feature(CPU_FTR_SMT)) {
+		for_each_present_cpu(i) {
+			if (i % 2 == 0)
+				/*
+				 * Even-numbered logical cpus correspond to
+				 * primary threads.
+				 */
+				cpu_set(i, of_spin_map);
+		}
+	} else {
+		of_spin_map = cpu_present_map;
+	}
Yes, we wrote code like that for CBE, but that doesn't mean it's good
enough to copy it ;-)

Relying on the cpu number to mean something specific is bad, and
we're trying to get rid of that in other places, so you should
not introduce it again here.
+#define CPU_FTRS_XENON ((CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB | \
+	    CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2 | \
+	    CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
+	    CPU_FTR_CTRL )&~CPU_FTR_16M_PAGE)
+// | CPU_FTR_PAUSE_ZERO | CPU_FTR_CI_LARGE_PAGE  /* we need to setup large
If the CPU doesn't do 16M pages, you need to make sure that CPU_FTRS_ALWAYS
is adapted properly.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help