Benjamin Herrenschmidt wrote:
On Tue, 2006-10-31 at 21:04 +0100, Nicolas DET wrote:
Here is is ;-).
As my mailer can insert a file, I copy paste. I hope it's still ok...
No, your patch got wrapped. Its damaged. I see you used Thunderbird. I
have no experience with sending patches with it, so I don't know what
the trick is to have them undamaged. With evolution, the trick is to use
the pre-defined style "preformat".
I'll figure uot something...
Anyway, minor comments, we're getting there...
Everything fixed.
+
+define_machine(efika)
+{
+ .name = EFIKA_PLATFORM_NAME,
+ .probe = efika_probe,
+ .setup_arch = efika_setup_arch,
+ .init = efika_init,
+ .show_cpuinfo = efika_show_cpuinfo,
+ .init_IRQ = efika_init_IRQ,
+ .get_irq = mpc52xx_get_irq,
+ .restart = rtas_restart,
+ .power_off = rtas_power_off,
+ .halt = rtas_halt,
+ .set_rtc_time = rtas_set_rtc_time,
+ .get_rtc_time = rtas_get_rtc_time,
+ .progress = rtas_progress,
+ .get_boot_time = rtas_get_boot_time,
+ .calibrate_decr = generic_calibrate_decr,
+ .phys_mem_access_prot = pci_phys_mem_access_prot,
+ .pcibios_fixup = efika_pciirq_map,
+};
The later can go away if you apply the patch I posted last week
[PATCH] Powerpc: Make pci_read_irq_line the default: on mpc7448hpc2
board. First.
Ok.
The whole function is not needed. Just apply my other patch first.
Compiling...
+ default y
+
+config PPC_EFIKA
+ bool "bPlan Efika 5k2. MPC5200B based computer"
+ depends on PPC_MULTIPLATFORM && PPC32
+ select PPC_RTAS
+ select RTAS_PROC
+ select PPC_MPC52xx
+ select PPC_MPC52xx_PIC
+ default y
+
config PPC_PMAC
bool "Apple PowerMac based machines"
depends on PPC_MULTIPLATFORM
PIC patch separate please.
Ok.
+/*
+ * Critial interrupt irq_chip
+*/
+static void mpc52xx_crit_mask(unsigned int virq)
+{
+ int irq;
+ int l2irq;
+
+ irq = irq_map[virq].hwirq;
+ l2irq = (irq & MPC52xx_IRQ_L2_MASK) >> MPC52xx_IRQ_L2_OFFSET;
+
+ pr_debug("%s: irq=%x, l2=%d\n", __func__, irq, l2irq);
+
+ BUG_ON(l2irq != 0);
+
+ io_be_clrbit(&intr->ctrl, 11);
+}
I'm not sure you understood my previous comment... any reason why crit
and mainirq are two different sets of functions and a different level 1
since crit is just basically mainirq 0 ? And main & mainirq, on the
contrary, should be different L1s since they are ... different :)
In my opinion, as it reflects a bit better hwow the hw itself is
architectured (critical, main, peripheral...) it's better to do it like
this. I do not wish to change this. Moreover, it's yet working pretty well.
+ mpc52xx_irqhost =
+ irq_alloc_host(IRQ_HOST_MAP_LINEAR, 0xd0,
+ &mpc52xx_irqhost_ops, -1);
I would prefer that 0xd0 to be a symbolic constant in the .h
Ok. will be done
+ if (mpc52xx_irqhost)
+ mpc52xx_irqhost->host_data = (void *)find_mpc52xx_picnode();
Casting to void * is fairly useless :)
Removed.
+#ifdef CONFIG_PCI
+#define _IO_BASE isa_io_base
+#define _ISA_MEM_BASE isa_mem_base
+#define PCI_DRAM_OFFSET pci_dram_offset
+#else
+#define _IO_BASE 0
+#define _ISA_MEM_BASE 0
+#define PCI_DRAM_OFFSET 0
+#endif
I told you several times to remove the above. The whole thing is
duplicate of io.h.
The fact that the former has a special case for CONFIG_PPC_MPC52xx is
bogus in the first place... you might want to turn -that- into a
It normaly does not compile if I remove it as state earlier. I'll remove
them and fixed the compile issue.
if defined(CONFIG_PPC_MPC52xx) && !defined(CONFIG_PPC_MERGE)
+/*
======================================================================== */
+/* Main registers/struct addresses
*/
+/*
======================================================================== */
+
+/* MBAR position */
+#define MPC52xx_MBAR 0xf0000000 /* Phys address */
+#define MPC52xx_MBAR_VIRT 0xf0000000 /* Virt address */
+#define MPC52xx_MBAR_SIZE 0x00010000
+
+#define MPC52xx_PA(x) ((phys_addr_t)(MPC52xx_MBAR + (x)))
+#define MPC52xx_VA(x) ((void __iomem *)(MPC52xx_MBAR_VIRT + (x)))
The above definitions are all bogus for arch/powerpc, just remove them.
Ok.
+/*
+ * 24 peripherals ints
+ * + 16 mains ints
+ * + 4 crit
+ * + 16 bestcomm task
+ * = 64
+*/
+#define MPC52xx_IRQ_MAXCOUNT (64)
The above is both not correct anymore and not used. Please fix it and
use it instead of hard coding.
Will be removed and replace by another define to reflect the highest
virq (0xd0).
#define MPC52xx_IRQ_MACVIRQ (0xd0)
sounds ok ?
+static inline struct device_node *find_mpc52xx_picnode(void)
+{
+ return of_find_compatible_node(NULL, "interrupt-controller",
+ "mpc5200-pic");
+}
Any reason why you need that inline since it's not used anywhere else
but the PIC code ? Just put that of_find_compatible_node() statement in
the .c and be done with it.
Done.
+ /* Matching of PSC function */
+struct mpc52xx_psc_func {
+ int id;
+ char *func;
+};
+extern int mpc52xx_match_psc_function(int psc_idx, const char *func);
+extern struct mpc52xx_psc_func mpc52xx_psc_functions[];
+ /* This array is to be defined in platform file */
The above doesn't look like it should migrate to arch/powerpc... what is
it supposed to be ?
Removed.
I'll re submit the patch as soon as 'done, compiled, tested'.
Bye
begin:vcard
fn:Nicolas DET ( bplan GmbH )
n:DET;Nicolas
org:bplan GmbH
adr:;;;;;;Germany
email;internet:[EMAIL PROTECTED]
title:Software Entwicklung
tel;work:+49 6171 9187 - 31
x-mozilla-html:FALSE
url:http://www.bplan-gmbh.de
version:2.1
end:vcard
_______________________________________________
Linuxppc-embedded mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-embedded
