82xx: some 82xx platform hook functions can be shared by different boards
PATCH 1/3 82xx: some 82xx platform hook functions can be shared by different boards
Some mpc82xx platform hooks in mpc82xx_ads.c are actually not specific to ads board, they could be shared by different 82xx boards.
--- arch/powerpc/platforms/82xx/mpc82xx.c | 30 +++++++++++++++++++---- arch/powerpc/platforms/82xx/mpc82xx_ads.c | 38 +++++++----------------------- arch/powerpc/platforms/82xx/pq2ads.h | 2 - include/asm-powerpc/mpc8260.h | 5 +++ 4 files changed, 39 insertions(+), 36 deletions(-)
arch/powerpc/platforms/82xx/mpc82xx.c +25 -5
@@ -50,8 +50,6 @@#include <sysdev/fsl_soc.h> #include <sysdev/cpm2_pic.h> -#include "pq2ads.h" -static int __init get_freq (char *name,unsigned long *val) {struct device_node *cpu;
@@ -74,7 +72,7 @@return found; } -void __init m82xx_calibrate_decr (void ) +void __init mpc82xx_calibrate_decr (void ) { ppc_tb_freq =125000000 ;if (!get_freq ("bus-frequency" , &ppc_tb_freq)) {
@@ -88,7 +86,7 @@ "(not found)\n"); } -void mpc82xx_ads_show_cpuinfo (struct seq_file *m) +void mpc82xx_show_cpuinfo (struct seq_file *m) {uint pvid, svid, phid1;uint memsize = total_memory;
@@ -96,7 +94,7 @@ pvid =mfspr (SPRN_PVR ); svid =mfspr (SPRN_SVR ); -seq_printf (m,"Vendor\t\t: Freescale Semiconductor\n" ); +seq_printf (m,"Vendor\t\t: %s\n" ,CPUINFO_VENDOR );seq_printf (m,"Machine\t\t: %s\n" ,CPUINFO_MACHINE );seq_printf (m,"PVR\t\t: 0x%x\n" , pvid);seq_printf (m,"SVR\t\t: 0x%x\n" , svid);
@@ -108,3 +106,25 @@/* Display the amount of memory */ seq_printf (m,"Memory\t\t: %d MB\n" , memsize / (1024 *1024 )); } + +#define RMR_CSRE 0x00000001 + +void mpc82xx_restart (char *cmd) +{ + __volatile__ unsigned char dummy; + +local_irq_disable (); + ((cpm2_map_t *) cpm2_immr)->im_clkrst.car_rmr |=RMR_CSRE ; + +/* Clear the ME,EE,IR & DR bits in MSR to cause checkstop */ +mtmsr (mfmsr () & ~(MSR_ME |MSR_EE |MSR_IR |MSR_DR )); + dummy = ((cpm2_map_t *) cpm2_immr)->im_clkrst.res[0 ]; +printk ("Restart failed\n" ); +while (1 ) ; +} + +void mpc82xx_halt (void ) +{ +local_irq_disable (); +while (1 ) ; +}
arch/powerpc/platforms/82xx/mpc82xx_ads.c +9 -29
@@ -606,35 +606,15 @@return 1 ; } -#define RMR_CSRE 0x00000001 -static void m82xx_restart (char *cmd) -{ - __volatile__ unsigned char dummy; - -local_irq_disable (); - ((cpm2_map_t *) cpm2_immr)->im_clkrst.car_rmr |=RMR_CSRE ; - -/* Clear the ME,EE,IR & DR bits in MSR to cause checkstop */ -mtmsr (mfmsr () & ~(MSR_ME |MSR_EE |MSR_IR |MSR_DR )); - dummy = ((cpm2_map_t *) cpm2_immr)->im_clkrst.res[0 ]; -printk ("Restart failed\n" ); -while (1 ) ; -} - -static void m82xx_halt (void ) -{ -local_irq_disable (); -while (1 ) ; -} -define_machine (mpc82xx_ads) { - .name = "MPC82xx ADS", - .probe = mpc82xx_ads_probe, - .setup_arch = mpc82xx_ads_setup_arch, - .init_IRQ = mpc82xx_ads_pic_init, - .show_cpuinfo = mpc82xx_ads_show_cpuinfo, - .get_irq = cpm2_get_irq, - .calibrate_decr = m82xx_calibrate_decr, - .restart = m82xx_restart,.halt = m82xx_halt, + .name = "MPC82xx ADS", + .probe = mpc82xx_ads_probe, + .setup_arch = mpc82xx_ads_setup_arch, + .init_IRQ = mpc82xx_ads_pic_init, + .show_cpuinfo = mpc82xx_show_cpuinfo, + .get_irq = cpm2_get_irq, + .calibrate_decr = mpc82xx_calibrate_decr, + .restart = mpc82xx_restart, + .halt = mpc82xx_halt, };
arch/powerpc/platforms/82xx/pq2ads.h +0 -2
@@ -59,8 +59,6 @@#define SIU_INT_SCC4 ((uint)0x2b+CPM_IRQ_OFFSET)void m82xx_pci_init_irq (void ); -void mpc82xx_ads_show_cpuinfo (struct seq_file *); -void m82xx_calibrate_decr (void );#endif /* __MACH_ADS8260_DEFS */ #endif /* __KERNEL__ */
include/asm-powerpc/mpc8260.h +5 -0
@@ -19,6 +19,11 @@#include <platforms/82xx/m82xx_pci.h> #endif +extern void mpc82xx_show_cpuinfo (struct seq_file *); +extern void mpc82xx_calibrate_decr (void ); +extern void mpc82xx_restart (char *cmd); +extern void mpc82xx_halt (void ); +#endif /* CONFIG_8260 */ #endif /* !__ASM_POWERPC_MPC8260_H__ */ #endif /* __KERNEL__ */
arch/powerpc/platforms/82xx/mpc82xx.c line 100
Quoted 9 lines · Mark Zhan
arch/powerpc/platforms/82xx/mpc82xx.c +1
-1@@ -96,7 +94,7 @@ pvid =mfspr (SPRN_PVR ); svid =mfspr (SPRN_SVR ); -seq_printf (m,"Vendor\t\t: Freescale Semiconductor\n" ); +seq_printf (m,"Vendor\t\t: %s\n" ,CPUINFO_VENDOR );seq_printf (m,"Machine\t\t: %s\n" ,CPUINFO_MACHINE );seq_printf (m,"PVR\t\t: 0x%x\n" , pvid);seq_printf (m,"SVR\t\t: 0x%x\n" , svid);
This is a step in the wrong direction. CPUINFO_{VENDOR,MACHINE} comes from a platform specific header file, so you can not use these definitions in platform independent code without breaking multiplatform kernels.
One possible solution would be a platform specific show_cpuinfo() function that calls a generic 82xx version and passes in the two values. Even better would be to just dump whatever string you find in the /model property in the device tree.
Quoted 6 lines
arch/powerpc/platforms/82xx/mpc82xx.c +6
-0+ +#define RMR_CSRE 0x00000001 + +void mpc82xx_restart (char *cmd) +{ + __volatile__ unsigned char dummy;
arch/powerpc/platforms/82xx/mpc82xx.c line 124
Quoted 10 lines
arch/powerpc/platforms/82xx/mpc82xx.c +10
-0+ +local_irq_disable (); + ((cpm2_map_t *) cpm2_immr)->im_clkrst.car_rmr |=RMR_CSRE ; + +/* Clear the ME,EE,IR & DR bits in MSR to cause checkstop */ +mtmsr (mfmsr () & ~(MSR_ME |MSR_EE |MSR_IR |MSR_DR )); + dummy = ((cpm2_map_t *) cpm2_immr)->im_clkrst.res[0 ]; +printk ("Restart failed\n" ); +while (1 ) ; +}
I know you're just moving that code, but it looks horribly wrong nonetheless. cpm2_immr is an __iomem variable, so you must not dereference it but instead should use the in_8() macro to access it.
Once you get that right, you don't need the volatile variable any more.
arch/powerpc/platforms/82xx/mpc82xx.c line 130
Quoted 5 lines
arch/powerpc/platforms/82xx/mpc82xx.c +5
-0+void mpc82xx_halt (void ) +{ +local_irq_disable (); +while (1 ) ; +}
Here, as in the function above, there should at least be a cpu_relax() in the final loop. If the CPU has a nap functionality or something similar, that would be even better.
Arnd <><
Quoted 17 lines · Arnd Bergmann
arch/powerpc/platforms/82xx/mpc82xx.c line 100
Quoted 9 lines · Mark Zhan
arch/powerpc/platforms/82xx/mpc82xx.c +1
-1@@ -96,7 +94,7 @@ pvid =mfspr (SPRN_PVR ); svid =mfspr (SPRN_SVR ); -seq_printf (m,"Vendor\t\t: Freescale Semiconductor\n" ); +seq_printf (m,"Vendor\t\t: %s\n" ,CPUINFO_VENDOR );seq_printf (m,"Machine\t\t: %s\n" ,CPUINFO_MACHINE );seq_printf (m,"PVR\t\t: 0x%x\n" , pvid);seq_printf (m,"SVR\t\t: 0x%x\n" , svid);This is a step in the wrong direction. CPUINFO_{VENDOR,MACHINE} comes from a platform specific header file, so you can not use these definitions in platform independent code without breaking multiplatform kernels.
Quoted 5 lines
One possible solution would be a platform specific
show_cpuinfo()function that calls a generic 82xx version and passes in the two values. Even better would be to just dump whatever string you find in the /model property in the device tree.
OK, Got what you said. I will fix it.
Quoted 22 lines
Quoted 6 lines
arch/powerpc/platforms/82xx/mpc82xx.c +6
-0+ +#define RMR_CSRE 0x00000001 + +void mpc82xx_restart (char *cmd) +{ + __volatile__ unsigned char dummy;arch/powerpc/platforms/82xx/mpc82xx.c line 124
Quoted 10 lines
arch/powerpc/platforms/82xx/mpc82xx.c +10
-0+ +local_irq_disable (); + ((cpm2_map_t *) cpm2_immr)->im_clkrst.car_rmr |=RMR_CSRE ; + +/* Clear the ME,EE,IR & DR bits in MSR to cause checkstop */ +mtmsr (mfmsr () & ~(MSR_ME |MSR_EE |MSR_IR |MSR_DR )); + dummy = ((cpm2_map_t *) cpm2_immr)->im_clkrst.res[0 ]; +printk ("Restart failed\n" ); +while (1 ) ; +}I know you're just moving that code, but it looks horribly wrong nonetheless.
cpm2_immris an__iomemvariable, so you must not dereference it but instead should use thein_8()macro to access it.
Quoted 2 lines
Once you get that right, you don't need the volatile variable any more.
OK. will fix it.
Quoted 10 lines
arch/powerpc/platforms/82xx/mpc82xx.c line 130
Quoted 6 lines
arch/powerpc/platforms/82xx/mpc82xx.c +5
-0+void mpc82xx_halt (void ) +{ +local_irq_disable (); +while (1 ) ; +}Here, as in the function above, there should at least be a
cpu_relax()in the final loop. If the CPU has a nap functionality or something similar, that would be even better.
Not sure if mpc82xx has such kind of functionality. Based on the current definition of cpu_relax(), it is only meaningful for ppc64.
Quoted 2 lines
Arnd <><
Quoted 4 lines · Arnd Bergmann
This is a step in the wrong direction. CPUINFO_{VENDOR,MACHINE} comes from a platform specific header file, so you can not use these definitions in platform independent code without breaking multiplatform kernels.
My patchset just drops the vendor field, and uses ppc_md.name for the machine name. The vendor name can be included in the latter.
Quoted 7 lines
I know you're just moving that code, but it looks horribly wrong nonetheless.
cpm2_immris an__iomemvariable, so you must not dereference it but instead should use thein_8()macro to access it.Once you get that right, you don't need the volatile variable any more.
My patchset addresses this.
-Scott