82xx: SBCPQ2 board platform support
Subsystems (2)
PATCH 3/3 82xx: SBCPQ2 board platform support
This patch addes the powerpc support to Wind River SBC PowerQUICCII 82xx board.
--- arch/powerpc/boot/dts/sbcpq2.dts | 191 +++++++ arch/powerpc/configs/sbcpq2_defconfig | 917 ++++++++++++++++++++++++++++++++++ arch/powerpc/platforms/82xx/Kconfig | 10 arch/powerpc/platforms/82xx/Makefile | 5 arch/powerpc/platforms/82xx/sbcpq2.c | 306 +++++++++++ arch/powerpc/platforms/82xx/sbcpq2.h | 118 ++++ include/asm-powerpc/mpc8260.h | 4 7 files changed, 1549 insertions(+), 2 deletions(-)
arch/powerpc/platforms/82xx/Kconfig +10 -0
@@ -13,6 +13,16 @@ help This option enables support for the MPC8272 ADS board +config SBCPQ2 + bool "Wind River SBC PowerQUICCII 82xx" + select DEFAULT_UIMAGE + select 8260 + select CPM2 + select FSL_SOC + help + This option enables support for Wind River SBC PowerQUICCII 82xx, + which is a single-board computer with MPC82xx CPU. + endchoice config PQ2ADS
arch/powerpc/platforms/82xx/Makefile +3 -2
@@ -1,5 +1,6 @@ # # Makefile for the PowerPC 82xx linux kernel. # -obj-$(CONFIG_PPC_82xx) += mpc82xx.o -obj-$(CONFIG_MPC82xx_ADS) += mpc82xx_ads.o +obj-$(CONFIG_PPC_82xx) += mpc82xx.o +obj-$(CONFIG_MPC82xx_ADS) += mpc82xx_ads.o +obj-$(CONFIG_SBCPQ2) += sbcpq2.o
arch/powerpc/platforms/82xx/sbcpq2.c new file +306 -0
306 lines not on this page · raw message
arch/powerpc/platforms/82xx/sbcpq2.h new file +118 -0
118 lines not on this page · raw message
include/asm-powerpc/mpc8260.h +4 -0
@@ -15,6 +15,10 @@#include <platforms/82xx/pq2ads.h> #endif +#ifdef CONFIG_SBCPQ2 +#include <platforms/82xx/sbcpq2.h> +#endif +#ifdef CONFIG_PCI_8260 #include <platforms/82xx/m82xx_pci.h> #endif
arch/powerpc/boot/dts/sbcpq2.dts new file +191 -0
191 lines not on this page · raw message
arch/powerpc/configs/sbcpq2_defconfig new file +917 -0
917 lines not on this page · raw message
Quoted 1 line · Mark Zhan
This patch addes the powerpc support to Wind River SBC PowerQUICCII 82xx board.
Hi Mark!
I've got lots of small comments about your code, but mostly it comes down to one problem: your new platform code is not able to coexist with other platforms because you hardcode information.
arch/powerpc/platforms/82xx/sbcpq2.c line 52
Quoted 12 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +12
-0+static struct resource m48t59_resources[] = { + { + .start =SBCPQ2_RTC_BASE , + .end =SBCPQ2_RTC_BASE +SBCPQ2_RTC_SIZE -1 , + .flags =IORESOURCE_MEM , + }, { + .start =SBCPQ2_M48T59_IRQ , + .end =SBCPQ2_M48T59_IRQ , + .flags =IORESOURCE_IRQ , + }, + { }, +};
This is the kind of information that belongs into the device tree, not hardcoded into the board code.
arch/powerpc/platforms/82xx/sbcpq2.c line 70
Quoted 6 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +6
-0+/** +* sbcpq2_pdev_init - Register the platform device for sbcpq2 board +*/ +static int __init sbcpq2_platdev_init (void ) +{ +struct irq_desc *desc = irq_desc +SBCPQ2_M48T59_IRQ ;
same for the interrupt number. Worse, this looks broken because the descriptor array describes virtual interrupt numbers, while SBCPQ2_M48T59_IRQ must be a physical number. These are often the same, but there is no guarantee.
In order to get a virtual interrupt number for a given device, you need to call irq_of_parse_and_map().
Quoted 24 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +22
-0+/* Install a dummy irq chip for M48T59 RTC irq */ +if (desc->chip == &no_irq_chip) +set_irq_handler (SBCPQ2_M48T59_IRQ , desc->handle_irq); + +/* Register all platform devices for sbcpq2 */ +platform_add_devices (sbcpq2_devices,ARRAY_SIZE (sbcpq2_devices)); +return 0 ; +} +arch_initcall (sbcpq2_platdev_init); +/* +* For SBCPQ2 board, the interrupt of M48T59 RTC chip +* will generate a machine check exception. We use a +* fake irq to give the platform machine_check_exception() hook +* a chance to call the driver ISR. If IRQ_HANDLED is returned, +* then we will survive from the machine check exception. +*/ +static int sbcpq2_mach_check (struct pt_regs *regs) +{ +int recover =0 ; +struct irq_desc *desc = irq_desc +SBCPQ2_M48T59_IRQ ; + +struct irqaction *action = desc->action;
arch/powerpc/platforms/82xx/sbcpq2.c
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +10
-0+ +while (action && (action->dev_id != &m48t59_rtc)) + action = action->next; + +/* Try to call m48t59 RTC driver ISR */ +if (action && action->handler) + recover = action->handler (SBCPQ2_M48T59_IRQ , &m48t59_rtc); + +return recover; +}
What you do here looks really scary, but maybe I'm just misunderstanding it completely. Why don't you just register your rtc handler function as the machine check handler instead of going through various indirections?
arch/powerpc/platforms/82xx/sbcpq2.c line 194
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +10
-0+static void __init sbcpq2_init_IRQ (void ) +{ +struct device_node *np; +struct resource res; + + np =of_find_compatible_node (NULL ,"cpm-pic" ,"CPM2" ); +if (np ==NULL ) { +printk (KERN_ERR "PIC init: can not find cpm-pic node\n" ); +return ; + }
This looks like your device tree is wrong. Shouldn't the interrupt controller have device_type="interrupt-controller" and a specific compatible property instead of having the name in the device_type?
arch/powerpc/platforms/82xx/sbcpq2.c line 218
Quoted 4 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +4
-0+static void __init sbcpq2_setup_arch (void ) +{ +struct device_node *np; +volatile memctl_cpm2_t *mc;
not volatile, but __iomem!
Quoted 5 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +5
-0+unsigned char * eeprom_base; +int i =0 ; + +#ifdef CONFIG_CPM2 +cpm2_reset ();
arch/powerpc/platforms/82xx/sbcpq2.c line 233
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +10
-0+#endif + +/* +* Make sure that we have the right CS# setting +*/ + mc = &cpm2_immr->im_memctl; + +/* Boot Flash is the on-board flash */ + mc->memc_br0 = (SBCPQ2_BOOT_FLASH_BASE &0xFFFF8000 ) |0x0801 ; + mc->memc_or0 =0xFFE00896 ;
consequently, this needs to use out_be32 or similar. Where does SBCPQ2_BOOT_FLASH_BASE come from? Shouldn't that be set up by the boot loader to match the device tree?
arch/powerpc/platforms/82xx/sbcpq2.c line 260
Quoted 3 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +3
-0+ model = (char *)of_get_property (np,"model" ,NULL ); +if (!model) +continue ;
The cast is not needed here.
arch/powerpc/platforms/82xx/sbcpq2.c line 268
Quoted 8 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +8
-0+ + id =of_get_property (np,"device-id" ,NULL ); +if (!id) +continue ; + + macaddr = (unsigned char *)of_get_mac_address (np); +if (!macaddr) +continue ;
or here.
arch/powerpc/platforms/82xx/sbcpq2.c line 274
Quoted 5 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +5
-0+if (strstr (model,"FCC" )) + eeprom_ofs =SBCPQ2_FCC1_MACADDR_OFS ; +else if (strstr (model,"SCC" )) + eeprom_ofs =SBCPQ2_SCC1_MACADDR_OFS ; + eeprom_ofs += ((*id) -1 ) *6 ;
of_device_is_compatible()
arch/powerpc/platforms/82xx/sbcpq2.c line 277
Quoted 2 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +2
-0+for (j =0 ; j <6 ; j++) + *(macaddr + j) = *(eeprom_base + eeprom_ofs + j);
in_8().
Quoted 5 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +5
-0+ } +iounmap (eeprom_base); +} + +/*
arch/powerpc/platforms/82xx/sbcpq2.c line 292
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +10
-0+* Called very early, device-tree isn't unflattened +*/ +static int __init sbcpq2_probe (void ) +{ +/* We always match for now, eventually we should look at +* the flat dev tree to ensure this is the board we are +* supposed to run on +*/ +return 1 ; +}
Don't write why the code is wrong -- just fix it.
arch/powerpc/platforms/82xx/sbcpq2.h line 20
Quoted 3 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +3
-0+/* For our show_cpuinfo hooks. */ +#define CPUINFO_VENDOR "Wind River" +#define CPUINFO_MACHINE "SBC PowerQUICCII 82xx"
Not in a header file please.
Quoted 47 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +47
-0+/* +* Wind River SBC PowerQUICCII 82xx Physical Memory Map (CS0 for OnBoard Flash) +* +* 0x00000000 - 0x07FFFFFF CS2, 128 MB DIMM SDRAM +* 0x08000000 - 0x0FFFFFFF CS3, 128 MB DIMM SDRAM +* 0x12000000 - 0x12100000 CS8, ATM +* 0x20000000 - 0x20FFFFFF CS4, 16 MB Local Bus SDRAM +* 0x21000000 - 0x21001FFF CS7, Control EPLD +* 0x22000000 - 0x22001FFF CS5, 8KB EEPROM +* 0x22002000 - 0x22003FFF CS5, visionPORT +* 0x22004000 - 0x22005FFF CS5, User Switches +* 0x22006000 - 0x22007FFF CS5, STATUS +* 0x22008000 - 0x22009FFF CS5, i8259 interrupt controller +* 0x2200A000 - 0x2200BFFF CS5, LED (Seven Segment Display) +* 0x80000000 - 0x80001FFF CS11, RTC +* 0xE0000000 - 0xE3FFFFFF CS6, 64 MB DIMM Flash +* 0xE4000000 - 0xE7FFFFFF CS1, 64 MB DIMM Flash +* 0xFE000000 - 0xFFFFFFFF CS0, 2 MB Boot Flash +* 0xF0000000 - 0xF0020000 MPC82xx Internal Registers Space +*/ +#define SBCPQ2_SDRAM_BASE 0x00000000 +#define SBCPQ2_SDRAM_SIZE 0x10000000 + +#define SBCPQ2_LOCAL_SDRAM_BASE 0x20000000 +#define SBCPQ2_LOCAL_SDRAM_SIZE 0x1000000 + +#define SBCPQ2_EPLD_BASE 0x21000000 +#define SBCPQ2_EPLD_SIZE 0x2000 + +#define SBCPQ2_EEPROM_BASE 0x22000000 +#define SBCPQ2_EEPROM_SIZE 0x2000 + +/* User Switches SW5 */ +#define SBCPQ2_USER_SW_BASE 0x22004000 +#define SBCPQ2_USER_SW_SIZE 0x2000 + +#define SBCPQ2_STATUS_BASE 0x22006000 +#define SBCPQ2_STATUS_SIZE 0x2000 + +#define SBCPQ2_I8259_BASE 0x22008000 +#define SBCPQ2_I8259_SIZE 0x2000 + +/* Seven Segment Display LED D46 */ +#define SBCPQ2_LED_BASE 0x2200A000 +#define SBCPQ2_LED_SIZE 0x2000 + +#define SBCPQ2_RTC_BASE 0x80000000
arch/powerpc/platforms/82xx/sbcpq2.h line 78
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +10
-0+#define SBCPQ2_RTC_SIZE 0x2000 + +#define SBCPQ2_BOOT_FLASH_BASE 0xFE000000 +#define SBCPQ2_BOOT_FLASH_SIZE 0x00200000 + +#define SBCPQ2_DIMM_FLASH_BASE 0xE0000000 +#define SBCPQ2_DIMM_FLASH_SIZE 0x04000000 + +#define CPM_MAP_ADDR 0xF0000000 +#define CPM_IRQ_OFFSET 0
All this is in the device tree already, so don't duplicate it here.
arch/powerpc/platforms/82xx/sbcpq2.h line 86
Quoted 7 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +7
-0+/* +* The offset of ethernet MAC addr within EEPROM +*/ +#define SBCPQ2_FCC1_MACADDR_OFS 0x60 +#define SBCPQ2_FCC2_MACADDR_OFS 0x66 +#define SBCPQ2_FCC3_MACADDR_OFS 0x72 +#define SBCPQ2_SCC1_MACADDR_OFS 0x78
Likewise, the mac address is in the device tree, so no need to tell the kernel how to read it.
arch/powerpc/platforms/82xx/sbcpq2.h line 108
Quoted 13 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +13
-0+/* +* The following IRQs are routed to i8259 PIC. +* +* NOTE: i8259 PIC is cascaded to SIU_INT_IRQ6 of CPM2 interrupt controller +*/ +#define SBCPQ2_PC_IRQA (NR_SIU_INTS+0) +#define SBCPQ2_PC_IRQB (NR_SIU_INTS+1) +#define SBCPQ2_MPC185_IRQ (NR_SIU_INTS+2) +#define SBCPQ2_ATM_IRQ (NR_SIU_INTS+3) +#define SBCPQ2_PIRQA (NR_SIU_INTS+4) +#define SBCPQ2_PIRQB (NR_SIU_INTS+5) +#define SBCPQ2_PIRQC (NR_SIU_INTS+6) +#define SBCPQ2_PIRQD (NR_SIU_INTS+7)
Again, these are in the device tree, so don't put them here.
arch/powerpc/platforms/82xx/sbcpq2.h line 116
Quoted 7 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +7
-0+/* cpm serial driver works with constants below */ +#define SIU_INT_SMC1 ((uint)0x04+CPM_IRQ_OFFSET) +#define SIU_INT_SMC2 ((uint)0x05+CPM_IRQ_OFFSET) +#define SIU_INT_SCC1 ((uint)0x28+CPM_IRQ_OFFSET) +#define SIU_INT_SCC2 ((uint)0x29+CPM_IRQ_OFFSET) +#define SIU_INT_SCC3 ((uint)0x2a+CPM_IRQ_OFFSET) +#define SIU_INT_SCC4 ((uint)0x2b+CPM_IRQ_OFFSET)
What are these for? If you need them in the device driver, just put them in there, not in a header file. Also, you should make sure not to pollute the global name space, so they should be named SBCPQ2_SIU_INT_* to make it clear that they are board specific.
include/asm-powerpc/mpc8260.h line 20
Quoted 3 lines
include/asm-powerpc/mpc8260.h +3
-0+#ifdef CONFIG_SBCPQ2 +#include <platforms/82xx/sbcpq2.h> +#endif
Never put #ifdef around an #include.
Quoted 4 lines
+
#ifdefCONFIG_PCI_8260
#include <platforms/82xx/m82xx_pci.h>
#endif
Kill this #ifdef as well while you're there. If you get name space conflicts, just rename the symbols to make them unique.
arch/powerpc/boot/dts/sbcpq2.dts line 20
Quoted 6 lines
arch/powerpc/boot/dts/sbcpq2.dts new file +6
-0+/ { + model = "SBCPQ2"; + compatible = "mpc82xx"; + #address-cells = <1>; + #size-cells = <1>; + linux,phandle = <100>;
Don't put explicit phandles here. If you need a reference, do it symbolically.
Arnd <><
Hi Arnd,
Quoted 7 lines · Arnd Bergmann
Quoted 7 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +7
-0+static struct resource m48t59_resources[] = { + { + .start =SBCPQ2_RTC_BASE , + .end =SBCPQ2_RTC_BASE +SBCPQ2_RTC_SIZE -1 , + .flags =IORESOURCE_MEM , + }, { + .start =SBCPQ2_M48T59_IRQ ,
Quoted 9 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 52
Quoted 5 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +5
-0+ .end = SBCPQ2_M48T59_IRQ, + .flags = IORESOURCE_IRQ, + }, + { }, +};This is the kind of information that belongs into the device tree, not hardcoded into the board code.
ok, I will move them into device tree.
Quoted 45 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 70
Quoted 6 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +6
-0+/** +* sbcpq2_pdev_init - Register the platform device for sbcpq2 board +*/ +static int __init sbcpq2_platdev_init (void ) +{ +struct irq_desc *desc = irq_desc +SBCPQ2_M48T59_IRQ ;same for the interrupt number. Worse, this looks broken because the descriptor array describes virtual interrupt numbers, while
SBCPQ2_M48T59_IRQmust be a physical number. These are often the same, but there is no guarantee.In order to get a virtual interrupt number for a given device, you need to call
irq_of_parse_and_map().Quoted 30 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +28
-0+/* Install a dummy irq chip for M48T59 RTC irq */ +if (desc->chip == &no_irq_chip) +set_irq_handler (SBCPQ2_M48T59_IRQ , desc->handle_irq); + +/* Register all platform devices for sbcpq2 */ +platform_add_devices (sbcpq2_devices,ARRAY_SIZE (sbcpq2_devices)); +return 0 ; +} +arch_initcall (sbcpq2_platdev_init); +/* +* For SBCPQ2 board, the interrupt of M48T59 RTC chip +* will generate a machine check exception. We use a +* fake irq to give the platform machine_check_exception() hook +* a chance to call the driver ISR. If IRQ_HANDLED is returned, +* then we will survive from the machine check exception. +*/ +static int sbcpq2_mach_check (struct pt_regs *regs) +{ +int recover =0 ; +struct irq_desc *desc = irq_desc +SBCPQ2_M48T59_IRQ ; + +struct irqaction *action = desc->action; + +while (action && (action->dev_id != &m48t59_rtc)) + action = action->next; + +/* Try to call m48t59 RTC driver ISR */ +if (action && action->handler)
Quoted 9 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 103
Quoted 4 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +4
-0+ recover = action->handler (SBCPQ2_M48T59_IRQ , &m48t59_rtc); + +return recover; +}What you do here looks really scary, but maybe I'm just misunderstanding it completely. Why don't you just register your rtc handler function as the machine check handler instead of going through various indirections?
The rtc M48T59 driver is not specific to my board, it is probably used by other board. So I can't register the rtc intr handler as the mcheck exception handler. And in the other side, there are also other machine check sources, right?
So here I add a platform mcheck hook for rtc intr handler. Yeah, it is really scary and confusing:-)
Quoted 6 lines
Quoted 6 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +6
-0+static void __init sbcpq2_init_IRQ (void ) +{ +struct device_node *np; +struct resource res; + + np =of_find_compatible_node (NULL ,"cpm-pic" ,"CPM2" );
Quoted 9 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 194
Quoted 4 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +4
-0+if (np ==NULL ) { +printk (KERN_ERR "PIC init: can not find cpm-pic node\n" ); +return ; + }This looks like your device tree is wrong. Shouldn't the interrupt controller have device_type="interrupt-controller" and a specific compatible property instead of having the name in the
device_type?
Here, I just copy the codes from mpc82xx_ads, is there anything wrong?
Quoted 7 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 218
Quoted 4 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +4
-0+static void __init sbcpq2_setup_arch (void ) +{ +struct device_node *np; +volatile memctl_cpm2_t *mc;not volatile, but
__iomem!
Fixed.
Quoted 11 lines
Quoted 11 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +11
-0+unsigned char * eeprom_base; +int i =0 ; + +#ifdef CONFIG_CPM2 +cpm2_reset (); +#endif + +/* +* Make sure that we have the right CS# setting +*/ + mc = &cpm2_immr->im_memctl;
Quoted 8 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 233
Quoted 4 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +4
-0+ +/* Boot Flash is the on-board flash */ + mc->memc_br0 = (SBCPQ2_BOOT_FLASH_BASE &0xFFFF8000 ) |0x0801 ; + mc->memc_or0 =0xFFE00896 ;consequently, this needs to use
out_be32or similar. Where doesSBCPQ2_BOOT_FLASH_BASEcome from? Shouldn't that be set up by the boot loader to match the device tree?
Fixed. out_be32 is used. The reason why they are needed is because some legacy u-boot for this board probably was setting up the wrong memory map.
Quoted 6 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 260
Quoted 4 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +3
-0+ model = (char *)of_get_property (np,"model" ,NULL ); +if (!model) +continue ;The cast is not needed here.
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 268
Quoted 8 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +8
-0+ + id =of_get_property (np,"device-id" ,NULL ); +if (!id) +continue ; + + macaddr = (unsigned char *)of_get_mac_address (np); +if (!macaddr) +continue ;or here.
Both cast are removed.
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 274
Quoted 6 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +5
-0+if (strstr (model,"FCC" )) + eeprom_ofs =SBCPQ2_FCC1_MACADDR_OFS ; +else if (strstr (model,"SCC" )) + eeprom_ofs =SBCPQ2_SCC1_MACADDR_OFS ; + eeprom_ofs += ((*id) -1 ) *6 ;
of_device_is_compatible()
Quoted 4 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 277
Quoted 2 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +2
-0+for (j =0 ; j <6 ; j++) + *(macaddr + j) = *(eeprom_base + eeprom_ofs + j);
in_8().
OK. in_8() will be used.
Quoted 151 lines
Quoted 6 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +5
-0+ } +iounmap (eeprom_base); +} + +/* arch/powerpc/platforms/82xx/sbcpq2.c line 292
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +10
-0+* Called very early, device-tree isn't unflattened +*/ +static int __init sbcpq2_probe (void ) +{ +/* We always match for now, eventually we should look at +* the flat dev tree to ensure this is the board we are +* supposed to run on +*/ +return 1 ; +}Don't write why the code is wrong -- just fix it.
arch/powerpc/platforms/82xx/sbcpq2.h line 20
Quoted 3 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +3
-0+/* For our show_cpuinfo hooks. */ +#define CPUINFO_VENDOR "Wind River" +#define CPUINFO_MACHINE "SBC PowerQUICCII 82xx"Not in a header file please.
Quoted 47 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +47
-0+/* +* Wind River SBC PowerQUICCII 82xx Physical Memory Map (CS0 for OnBoard Flash) +* +* 0x00000000 - 0x07FFFFFF CS2, 128 MB DIMM SDRAM +* 0x08000000 - 0x0FFFFFFF CS3, 128 MB DIMM SDRAM +* 0x12000000 - 0x12100000 CS8, ATM +* 0x20000000 - 0x20FFFFFF CS4, 16 MB Local Bus SDRAM +* 0x21000000 - 0x21001FFF CS7, Control EPLD +* 0x22000000 - 0x22001FFF CS5, 8KB EEPROM +* 0x22002000 - 0x22003FFF CS5, visionPORT +* 0x22004000 - 0x22005FFF CS5, User Switches +* 0x22006000 - 0x22007FFF CS5, STATUS +* 0x22008000 - 0x22009FFF CS5, i8259 interrupt controller +* 0x2200A000 - 0x2200BFFF CS5, LED (Seven Segment Display) +* 0x80000000 - 0x80001FFF CS11, RTC +* 0xE0000000 - 0xE3FFFFFF CS6, 64 MB DIMM Flash +* 0xE4000000 - 0xE7FFFFFF CS1, 64 MB DIMM Flash +* 0xFE000000 - 0xFFFFFFFF CS0, 2 MB Boot Flash +* 0xF0000000 - 0xF0020000 MPC82xx Internal Registers Space +*/ +#define SBCPQ2_SDRAM_BASE 0x00000000 +#define SBCPQ2_SDRAM_SIZE 0x10000000 + +#define SBCPQ2_LOCAL_SDRAM_BASE 0x20000000 +#define SBCPQ2_LOCAL_SDRAM_SIZE 0x1000000 + +#define SBCPQ2_EPLD_BASE 0x21000000 +#define SBCPQ2_EPLD_SIZE 0x2000 + +#define SBCPQ2_EEPROM_BASE 0x22000000 +#define SBCPQ2_EEPROM_SIZE 0x2000 + +/* User Switches SW5 */ +#define SBCPQ2_USER_SW_BASE 0x22004000 +#define SBCPQ2_USER_SW_SIZE 0x2000 + +#define SBCPQ2_STATUS_BASE 0x22006000 +#define SBCPQ2_STATUS_SIZE 0x2000 + +#define SBCPQ2_I8259_BASE 0x22008000 +#define SBCPQ2_I8259_SIZE 0x2000 + +/* Seven Segment Display LED D46 */ +#define SBCPQ2_LED_BASE 0x2200A000 +#define SBCPQ2_LED_SIZE 0x2000 + +#define SBCPQ2_RTC_BASE 0x80000000arch/powerpc/platforms/82xx/sbcpq2.h line 78
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +10
-0+#define SBCPQ2_RTC_SIZE 0x2000 + +#define SBCPQ2_BOOT_FLASH_BASE 0xFE000000 +#define SBCPQ2_BOOT_FLASH_SIZE 0x00200000 + +#define SBCPQ2_DIMM_FLASH_BASE 0xE0000000 +#define SBCPQ2_DIMM_FLASH_SIZE 0x04000000 + +#define CPM_MAP_ADDR 0xF0000000 +#define CPM_IRQ_OFFSET 0All this is in the device tree already, so don't duplicate it here.
arch/powerpc/platforms/82xx/sbcpq2.h line 86
Quoted 7 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +7
-0+/* +* The offset of ethernet MAC addr within EEPROM +*/ +#define SBCPQ2_FCC1_MACADDR_OFS 0x60 +#define SBCPQ2_FCC2_MACADDR_OFS 0x66 +#define SBCPQ2_FCC3_MACADDR_OFS 0x72 +#define SBCPQ2_SCC1_MACADDR_OFS 0x78Likewise, the mac address is in the device tree, so no need to tell the kernel how to read it.
arch/powerpc/platforms/82xx/sbcpq2.h line 108
Quoted 13 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +13
-0+/* +* The following IRQs are routed to i8259 PIC. +* +* NOTE: i8259 PIC is cascaded to SIU_INT_IRQ6 of CPM2 interrupt controller +*/ +#define SBCPQ2_PC_IRQA (NR_SIU_INTS+0) +#define SBCPQ2_PC_IRQB (NR_SIU_INTS+1) +#define SBCPQ2_MPC185_IRQ (NR_SIU_INTS+2) +#define SBCPQ2_ATM_IRQ (NR_SIU_INTS+3) +#define SBCPQ2_PIRQA (NR_SIU_INTS+4) +#define SBCPQ2_PIRQB (NR_SIU_INTS+5) +#define SBCPQ2_PIRQC (NR_SIU_INTS+6) +#define SBCPQ2_PIRQD (NR_SIU_INTS+7)Again, these are in the device tree, so don't put them here.
arch/powerpc/platforms/82xx/sbcpq2.h line 116
Quoted 7 lines
arch/powerpc/platforms/82xx/sbcpq2.h new file +7
-0+/* cpm serial driver works with constants below */ +#define SIU_INT_SMC1 ((uint)0x04+CPM_IRQ_OFFSET) +#define SIU_INT_SMC2 ((uint)0x05+CPM_IRQ_OFFSET) +#define SIU_INT_SCC1 ((uint)0x28+CPM_IRQ_OFFSET) +#define SIU_INT_SCC2 ((uint)0x29+CPM_IRQ_OFFSET) +#define SIU_INT_SCC3 ((uint)0x2a+CPM_IRQ_OFFSET) +#define SIU_INT_SCC4 ((uint)0x2b+CPM_IRQ_OFFSET)What are these for? If you need them in the device driver, just put them in there, not in a header file. Also, you should make sure not to pollute the global name space, so they should be named SBCPQ2_SIU_INT_* to make it clear that they are board specific.
include/asm-powerpc/mpc8260.h line 20
Quoted 3 lines
include/asm-powerpc/mpc8260.h +3
-0+#ifdef CONFIG_SBCPQ2 +#include <platforms/82xx/sbcpq2.h> +#endif Never put #ifdef around an #include.
Quoted 4 lines
+
#ifdefCONFIG_PCI_8260
#include <platforms/82xx/m82xx_pci.h>
#endifKill this #ifdef as well while you're there. If you get name space conflicts, just rename the symbols to make them unique.
arch/powerpc/boot/dts/sbcpq2.dts line 20
Quoted 6 lines
arch/powerpc/boot/dts/sbcpq2.dts new file +6
-0+/ { + model = "SBCPQ2"; + compatible = "mpc82xx"; + #address-cells = <1>; + #size-cells = <1>; + linux,phandle = <100>;Don't put explicit phandles here. If you need a reference, do it symbolically.
Arnd <><
Quoted 32 lines · Mark Zhan
Hi Arnd,
Quoted 19 lines
Quoted 19 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +19
-0+/* +* For SBCPQ2 board, the interrupt of M48T59 RTC chip +* will generate a machine check exception. We use a +* fake irq to give the platform machine_check_exception() hook +* a chance to call the driver ISR. If IRQ_HANDLED is returned, +* then we will survive from the machine check exception. +*/ +static int sbcpq2_mach_check (struct pt_regs *regs) +{ +int recover =0 ; +struct irq_desc *desc = irq_desc +SBCPQ2_M48T59_IRQ ; + +struct irqaction *action = desc->action; + +while (action && (action->dev_id != &m48t59_rtc)) + action = action->next; + +/* Try to call m48t59 RTC driver ISR */ +if (action && action->handler)Quoted 9 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 103
Quoted 4 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +4
-0+ recover = action->handler (SBCPQ2_M48T59_IRQ , &m48t59_rtc); + +return recover; +}What you do here looks really scary, but maybe I'm just misunderstanding it completely. Why don't you just register your rtc handler function as the machine check handler instead of going through various indirections?
Quoted 7 lines
The rtc M48T59 driver is not specific to my board, it is probably used by other board. So I can't register the rtc intr handler as the mcheck exception handler. And in the other side, there are also other machine check sources, right?
So here I add a platform mcheck hook for rtc intr handler. Yeah, it is really scary and confusing:-)
I think it would really be easier to just make the m48t59 irq handler a global function, and keep it out of the regular interrupt logic.
Then your sbcpq2_mach_check() basically becomes trivial like
static int sbcpq2_mach_check(struct pt_regs *regs)
{
return m48t59_irq(NO_IRQ, NULL);
}This has the advantage that you don't need to wait for the m48t59 driver to be initialized first, instead you will just get a link failure it that driver is not already built into the kernel.
Quoted 16 lines
Quoted 6 lines
Quoted 6 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +6
-0+static void __init sbcpq2_init_IRQ (void ) +{ +struct device_node *np; +struct resource res; + + np =of_find_compatible_node (NULL ,"cpm-pic" ,"CPM2" );Quoted 9 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 194
Quoted 4 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +4
-0+if (np ==NULL ) { +printk (KERN_ERR "PIC init: can not find cpm-pic node\n" ); +return ; + }This looks like your device tree is wrong. Shouldn't the interrupt controller have device_type="interrupt-controller" and a specific compatible property instead of having the name in the
device_type?
Quoted 1 line
Here, I just copy the codes from
mpc82xx_ads, is there anything wrong?
I just checked the Recommended Practice document for interrupt mapping and it seems that it's ok. The interrupt controller needs to have an property named "interrupt-controller", but does not need a specific device_type. So it appears to be correct here.
Quoted 9 lines
Quoted 7 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 233
Quoted 3 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +3
-0+/* Boot Flash is the on-board flash */ + mc->memc_br0 = (SBCPQ2_BOOT_FLASH_BASE &0xFFFF8000 ) |0x0801 ; + mc->memc_or0 =0xFFE00896 ;consequently, this needs to use
out_be32or similar. Where doesSBCPQ2_BOOT_FLASH_BASEcome from? Shouldn't that be set up by the boot loader to match the device tree?Fixed.
out_be32is used.
btw, it would be good if you can run your code through the 'sparse' checker. It will warn about this type of problem. I think I saw all that you have added here, but I may have missed some, and sparse can also find other problems. Just install the tool as it comes with your distro and build the kernel with the 'C=1' make option.
Quoted 3 lines
The reason why they are needed is because some
legacy u-boot for this board probably was setting up the wrong memory map.
Hmm, will those legacy u-boot version be able to even boot this kernel? The device tree looks like it needs to have some variables set by u-boot, so I'd guess you don't need to worry about old versions that don't set those either.
Arnd <><
Quoted 14 lines
Quoted 7 lines
The rtc M48T59 driver is not specific to my board, it is probably used by other board. So I can't register the rtc intr handler as the mcheck exception handler. And in the other side, there are also other machine check sources, right?
So here I add a platform mcheck hook for rtc intr handler. Yeah, it is really scary and confusing:-)
I think it would really be easier to just make the m48t59 irq handler a global function, and keep it out of the regular interrupt logic.
Then your
sbcpq2_mach_check()basically becomes trivial likestatic int
sbcpq2_mach_check(struct pt_regs *regs)
Quoted 9 lines
{ return m48t59_irq(NO_IRQ, NULL); }This has the advantage that you don't need to wait for the m48t59 driver to be initialized first, instead you will just get a link failure it that driver is not already built into the kernel.
Since the rtc m48t59 driver has already gone into the -mm source tree, and I think, it is an ugly way to make the irq handler a global function:-)
If the driver is not built-in, and I still get the mach check exception, it will turn out that other factors are causing mach check exception, then just like my original codes, the kernel will not survive this machine check, right?
Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 194
Quoted 10 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +10
-0+static void __init sbcpq2_init_IRQ (void ) +{ +struct device_node *np; +struct resource res; + + np =of_find_compatible_node (NULL ,"cpm-pic" ,"CPM2" ); +if (np ==NULL ) { +printk (KERN_ERR "PIC init: can not find cpm-pic node\n" ); +return ; + }This looks like your device tree is wrong. Shouldn't the interrupt controller have device_type="interrupt-controller" and a specific compatible property instead of having the name in the
device_type?
Quoted 7 lines
Quoted 1 line
Here, I just copy the codes from
mpc82xx_ads, is there anything wrong?I just checked the Recommended Practice document for interrupt mapping and it seems that it's ok. The interrupt controller needs to have an property named "interrupt-controller", but does not need a specific
device_type. So it appears to be correct here.
Thanks a lot for your good adivce, I will try.
Quoted 16 lines
Quoted 9 lines
Quoted 7 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 233
Quoted 3 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +3
-0+/* Boot Flash is the on-board flash */ + mc->memc_br0 = (SBCPQ2_BOOT_FLASH_BASE &0xFFFF8000 ) |0x0801 ; + mc->memc_or0 =0xFFE00896 ;consequently, this needs to use
out_be32or similar. Where doesSBCPQ2_BOOT_FLASH_BASEcome from? Shouldn't that be set up by the boot loader to match the device tree?Fixed.
out_be32is used.btw, it would be good if you can run your code through the 'sparse' checker. It will warn about this type of problem. I think I saw all that you have added here, but I may have missed some, and sparse can also find other problems. Just install the tool as it comes with your distro and build the kernel with the 'C=1' make option.
Quoted 8 lines
Quoted 3 lines
The reason why they are needed is because some
legacy u-boot for this board probably was setting up the wrong memory map.Hmm, will those legacy u-boot version be able to even boot this kernel? The device tree looks like it needs to have some variables set by u-boot, so I'd guess you don't need to worry about old versions that don't set those either.
Actually, the current u-boot 1.2.0 for this board is ported from that legacy one, and I don't check the details to assure that u-boot's setup is all right. So here I just want to double assure everything goes correct.
Thanks
Mark Zhan
Quoted 9 lines · Mark Zhan
Since the rtc m48t59 driver has already gone into the -mm source tree, and I think, it is an ugly way to make the irq handler a global function:-)
If the driver is not built-in, and I still get the mach check exception, it will turn out that other factors are causing mach check exception, then just like my original codes, the kernel will not survive this machine check, right?
Ok, I missed the point that you also have real machine check events to care about.
One issue remains though: you still need to have a virtual irq number, not just make up a IRQ, as there is nothing that guarantees this to work.
Since you already create your own irq_host, you can probably just give the fake irq the number '1' here, which is local to that host. Then you call irq_create_mapping() to create the virtual interrupt number that you pass to the platform_device.
Arnd <><
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
arch/powerpc/platforms/82xx/sbcpq2.c line 194
Quoted 5 lines
arch/powerpc/platforms/82xx/sbcpq2.c new file +5
-0+ np =of_find_compatible_node (NULL ,"cpm-pic" ,"CPM2" ); +if (np ==NULL ) { +printk (KERN_ERR "PIC init: can not find cpm-pic node\n" ); +return ; + }This looks like your device tree is wrong. Shouldn't the interrupt controller have device_type="interrupt-controller" and a specific compatible property instead of having the name in the
device_type?
So, I looked it up :-)
The device_type should specify the specific interrupt controller programming model, because a real OF typically needs to know the meaning of the interrupt specifiers; also, when the interrupt binding recommended practice isn't used, it is needed to know the #interrupt-cells (which is implicit from the device_type in that case).
If there is no defined binding for your interrupt controller, just don't put a device_type in your tree at all, for flat device trees.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
Here, I just copy the codes from
mpc82xx_ads, is there anything wrong?I just checked the Recommended Practice document for interrupt mapping and it seems that it's ok. The interrupt controller needs to have an property named "interrupt-controller", but does not need a specific
device_type. So it appears to be correct here.
Please also look at the base specifications, not just the imap thing. But you reached the correct conclusion.
Segher