[PATCH 1/2] POWERPC: Remove global CPM2 mappings

[PATCH 1/2] POWERPC: Remove global CPM2 mappings

[Vitaly Bordug]

Gets rid or direct IMMR accesses/dereferences for PQ2 SoC targets and
relevant drivers. 

Previous approach was to setup the specific immr offset early and make whole immr structure open via global variable/define. Now in order to access some specific immr part it has to be mapped. Along with the clarity, it deprecates many board-specific ifdefs from common drivers code as well as platform header files.

Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>
---

 arch/powerpc/platforms/82xx/mpc82xx_ads.c |    6 +++-
 arch/powerpc/platforms/82xx/pq2ads.h      |    4 ---
 arch/powerpc/sysdev/cpm2_common.c         |   17 +-----------
 arch/powerpc/sysdev/fsl_soc.c             |   42 ++++++++++++++++++++++++-----
 arch/powerpc/sysdev/fsl_soc.h             |    1 +
 drivers/net/fs_enet/fs_enet-main.c        |    8 +++---
 drivers/net/fs_enet/mac-fcc.c             |    3 +-
 drivers/serial/cpm_uart/cpm_uart_cpm2.h   |   12 ++++----
 include/asm-powerpc/fs_pd.h               |   14 ++++------
 9 files changed, 58 insertions(+), 49 deletions(-)

diff --git a/arch/powerpc/platforms/82xx/mpc82xx_ads.c b/arch/powerpc/platforms/82xx/mpc82xx_ads.c
index 47cb09f..4902e70 100644
--- a/arch/powerpc/platforms/82xx/mpc82xx_ads.c
+++ b/arch/powerpc/platforms/82xx/mpc82xx_ads.c
@@ -46,6 +46,7 @@
 #include <asm/cpm2.h>
 #include <asm/udbg.h>
 #include <asm/i8259.h>
+#include <asm/fs_pd.h>
 #include <linux/fs_enet_pd.h>
 
 #include <sysdev/fsl_soc.h>
@@ -615,13 +616,14 @@ static int __init mpc82xx_ads_probe(void)
 static void m82xx_restart(char *cmd)
 {
 	__volatile__ unsigned char dummy;
+	car_cpm2_t *clk = cpm2_map(im_clkrst);
 
 	local_irq_disable();
-	((cpm2_map_t *) cpm2_immr)->im_clkrst.car_rmr |= RMR_CSRE;
+	setbits32(&clk->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];
+	dummy = in_8(&clk->res[0]);
 	printk("Restart failed\n");
 	while (1) ;
 }
diff --git a/arch/powerpc/platforms/82xx/pq2ads.h b/arch/powerpc/platforms/82xx/pq2ads.h
index 5b5cca6..f0cd179 100644
--- a/arch/powerpc/platforms/82xx/pq2ads.h
+++ b/arch/powerpc/platforms/82xx/pq2ads.h
@@ -29,10 +29,6 @@
 #define CPUINFO_VENDOR		"Freescale Semiconductor"
 #define CPUINFO_MACHINE		"PQ2 ADS PowerPC"
 
-/* Backword-compatibility stuff for the drivers */
-#define CPM_MAP_ADDR		((uint)0xf0000000)
-#define CPM_IRQ_OFFSET 0
-
 /* The ADS8260 has 16, 32-bit wide control/status registers, accessed
  * only on word boundaries.
  * Not all are used (yet), or are interesting to us (yet).
diff --git a/arch/powerpc/sysdev/cpm2_common.c b/arch/powerpc/sysdev/cpm2_common.c
index ec26599..229a4c0 100644
--- a/arch/powerpc/sysdev/cpm2_common.c
+++ b/arch/powerpc/sysdev/cpm2_common.c
@@ -45,31 +45,16 @@
 #include <sysdev/fsl_soc.h>
 
 static void cpm2_dpinit(void);
-cpm_cpm2_t	*cpmp;		/* Pointer to comm processor space */
-
-/* We allocate this here because it is used almost exclusively for
- * the communication processor devices.
- */
-cpm2_map_t *cpm2_immr;
 intctl_cpm2_t *cpm2_intctl;
 
-#define CPM_MAP_SIZE	(0x40000)	/* 256k - the PQ3 reserve this amount
-					   of space for CPM as it is larger
-					   than on PQ2 */
-
 void
 cpm2_reset(void)
 {
-	cpm2_immr = (cpm2_map_t *)ioremap(CPM_MAP_ADDR, CPM_MAP_SIZE);
 	cpm2_intctl = cpm2_map(im_intctl);
 
 	/* Reclaim the DP memory for our use.
 	 */
 	cpm2_dpinit();
-
-	/* Tell everyone where the comm processor resides.
-	 */
-	cpmp = &cpm2_immr->im_cpm;
 }
 
 /* Set a baud rate generator.  This needs lots of work.  There are
@@ -234,7 +219,7 @@ static void cpm2_dpinit(void)
 {
 	spin_lock_init(&cpm_dpmem_lock);
 
-	im_dprambase = ioremap(CPM_MAP_ADDR, CPM_DATAONLY_BASE + CPM_DATAONLY_SIZE);
+	im_dprambase = ioremap(get_cpmbase(), FCC_MEM_OFFSET(4));
 
 	/* initialize the info header */
 	rh_init(&cpm_dpmem_info, 1,
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 8a123c7..3c09b28 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -35,7 +35,7 @@
 #include <asm/prom.h>
 #include <sysdev/fsl_soc.h>
 #include <mm/mmu_decl.h>
-#include <asm/cpm2.h>
+#include <asm/fs_pd.h>
 
 extern void init_fcc_ioports(struct fs_platform_info*);
 extern void init_fec_ioports(struct fs_platform_info*);
@@ -66,6 +66,30 @@ EXPORT_SYMBOL(get_immrbase);
 
 #if defined(CONFIG_CPM2) || defined(CONFIG_8xx)
 
+static phys_addr_t cpmbase = -1;
+
+phys_addr_t get_cpmbase(void)
+{
+	struct device_node *cpm;
+
+	if (cpmbase != -1)
+		return cpmbase;
+
+	cpm = of_find_node_by_type(NULL, "cpm");
+	if (cpm) {
+		unsigned int size;
+		const void *prop = get_property(cpm, "reg", &size);
+
+		if (prop)
+			cpmbase = of_translate_address(cpm, prop);
+		of_node_put(cpm);
+	};
+
+	return cpmbase;
+}
+
+EXPORT_SYMBOL(get_cpmbase);
+
 static u32 brgfreq = -1;
 
 u32 get_brgfreq(void)
@@ -115,6 +139,7 @@ u32 get_baudrate(void)
 }
 
 EXPORT_SYMBOL(get_baudrate);
+
 #endif /* CONFIG_CPM2 */
 
 static int __init gfar_mdio_of_init(void)
@@ -593,6 +618,7 @@ static int __init fs_enet_of_init(void)
 	struct platform_device *fs_enet_dev;
 	struct resource res;
 	int ret;
+	int configured = 0;
 
 	for (np = NULL, i = 0;
 	     (np = of_find_compatible_node(np, "network", "fs_enet")) != NULL;
@@ -693,14 +719,15 @@ static int __init fs_enet_of_init(void)
 			fs_enet_data.bus_id = (char*)&bus_id[(*id)];
 			fs_enet_data.init_ioports = init_fcc_ioports;
 
-			mdio_bb_prop = of_get_property(phy, "bitbang", NULL);
-			if (mdio_bb_prop) {
+			mdio_bb_prop = get_property(phy, "bitbang", NULL);
+			if (mdio_bb_prop && !configured) {
 				struct platform_device *fs_enet_mdio_bb_dev;
 				struct fs_mii_bb_platform_info fs_enet_mdio_bb_data;
+				struct io_port *io = cpm2_map(im_ioport);
 
 				fs_enet_mdio_bb_dev =
 					platform_device_register_simple("fsl-bb-mdio",
-							i, NULL, 0);
+							res.start, NULL, 0);
 				memset(&fs_enet_mdio_bb_data, 0,
 						sizeof(struct fs_mii_bb_platform_info));
 				fs_enet_mdio_bb_data.mdio_dat.bit =
@@ -723,11 +750,11 @@ static int __init fs_enet_of_init(void)
 				fs_enet_mdio_bb_data.irq[31] = -1;
 
 				fs_enet_mdio_bb_data.mdio_dat.offset =
-					(u32)&cpm2_immr->im_ioport.iop_pdatc;
+					(u32)&io->iop_pdatc;
 				fs_enet_mdio_bb_data.mdio_dir.offset =
-					(u32)&cpm2_immr->im_ioport.iop_pdirc;
+					(u32)&io->iop_pdirc;
 				fs_enet_mdio_bb_data.mdc_dat.offset =
-					(u32)&cpm2_immr->im_ioport.iop_pdatc;
+					(u32)&io->iop_pdatc;
 
 				ret = platform_device_add_data(
 						fs_enet_mdio_bb_dev,
@@ -735,6 +762,7 @@ static int __init fs_enet_of_init(void)
 						sizeof(struct fs_mii_bb_platform_info));
 				if (ret)
 					goto unreg;
+				configured++;
 			}
 
 			of_node_put(phy);
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 04e145b..debcfdb 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -5,6 +5,7 @@
 #include <asm/mmu.h>
 
 extern phys_addr_t get_immrbase(void);
+extern phys_addr_t get_cpmbase(void);
 extern u32 get_brgfreq(void);
 extern u32 get_baudrate(void);
 
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index 4a05c14..5bd99c6 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -1136,14 +1136,14 @@ static int setup_immap(void)
 
 #ifdef CONFIG_CPM1
 	paddr = IMAP_ADDR;
-	size = 0x10000;	/* map 64K */
+	size = 0x10000; /* map 64K */
+	fs_enet_immap = ioremap(paddr, size);
 #endif
 
 #ifdef CONFIG_CPM2
-	paddr = CPM_MAP_ADDR;
-	size = 0x40000;	/* map 256 K */
+	fs_enet_immap = cpm2_map(im_cpm);
 #endif
-	fs_enet_immap = ioremap(paddr, size);
+
 	if (fs_enet_immap == NULL)
 		return -EBADF;	/* XXX ahem; maybe just BUG_ON? */
 
diff --git a/drivers/net/fs_enet/mac-fcc.c b/drivers/net/fs_enet/mac-fcc.c
index 8545e84..bd803ba 100644
--- a/drivers/net/fs_enet/mac-fcc.c
+++ b/drivers/net/fs_enet/mac-fcc.c
@@ -88,8 +88,7 @@ static inline int fcc_cr_cmd(struct fs_enet_private *fep, u32 mcn, u32 op)
 {
 	const struct fs_platform_info *fpi = fep->fpi;
 
-	cpm2_map_t *immap = fs_enet_immap;
-	cpm_cpm2_t *cpmp = &immap->im_cpm;
+	cpm_cpm2_t *cpmp = fs_enet_immap;
 	u32 v;
 	int i;
 
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.h b/drivers/serial/cpm_uart/cpm_uart_cpm2.h
index 1b3219f..34978fc 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm2.h
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.h
@@ -13,12 +13,12 @@
 #include <asm/cpm2.h>
 
 /* defines for IRQs */
-#define SMC1_IRQ	SIU_INT_SMC1
-#define SMC2_IRQ	SIU_INT_SMC2
-#define SCC1_IRQ	SIU_INT_SCC1
-#define SCC2_IRQ	SIU_INT_SCC2
-#define SCC3_IRQ	SIU_INT_SCC3
-#define SCC4_IRQ	SIU_INT_SCC4
+#define SMC1_IRQ	(CPM_IRQ_OFFSET + 0x04)
+#define SMC2_IRQ	(CPM_IRQ_OFFSET + 0x05)
+#define SCC1_IRQ	(CPM_IRQ_OFFSET + 0x28)
+#define SCC2_IRQ	(CPM_IRQ_OFFSET + 0x29)
+#define SCC3_IRQ	(CPM_IRQ_OFFSET + 0x2a)
+#define SCC4_IRQ	(CPM_IRQ_OFFSET + 0x2b)
 
 static inline void cpm_set_brg(int brg, int baud)
 {
diff --git a/include/asm-powerpc/fs_pd.h b/include/asm-powerpc/fs_pd.h
index c624915..77a44f9 100644
--- a/include/asm-powerpc/fs_pd.h
+++ b/include/asm-powerpc/fs_pd.h
@@ -17,16 +17,10 @@
 #ifdef CONFIG_CPM2
 #include <asm/cpm2.h>
 
-#if defined(CONFIG_8260)
-#include <asm/mpc8260.h>
-#elif defined(CONFIG_85xx)
-#include <asm/mpc85xx.h>
-#endif
-
 #define cpm2_map(member)						\
 ({									\
 	u32 offset = offsetof(cpm2_map_t, member);			\
-	void *addr = ioremap (CPM_MAP_ADDR + offset,			\
+	void *addr = ioremap (get_cpmbase() + offset,			\
 			      sizeof( ((cpm2_map_t*)0)->member));	\
 	addr;								\
 })
@@ -34,7 +28,7 @@
 #define cpm2_map_size(member, size)					\
 ({									\
 	u32 offset = offsetof(cpm2_map_t, member);			\
-	void *addr = ioremap (CPM_MAP_ADDR + offset, size);		\
+	void *addr = ioremap (get_cpmbase() + offset,			\
 	addr;								\
 })
 
@@ -73,4 +67,8 @@ static inline int uart_clock(void)
         return ppc_proc_freq;
 }
 
+#ifndef CPM_IRQ_OFFSET
+#define CPM_IRQ_OFFSET 0       /* Backword-compatibility stuff for the drivers */
+#endif
+
 #endif

[PATCH 2/2] POWERPC: Remove global CPM mappings

[Vitaly Bordug]

Gets rid or direct IMMR accesses/dereferences for PQ SoC targets and
relevant drivers.

Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>
---

 arch/powerpc/sysdev/commproc.c          |   13 +++----------
 drivers/net/fs_enet/fs_enet-main.c      |    7 +------
 drivers/net/fs_enet/mac-scc.c           |    2 +-
 drivers/serial/cpm_uart/cpm_uart_cpm1.c |    6 +++++-
 drivers/serial/cpm_uart/cpm_uart_cpm1.h |    2 +-
 include/asm-powerpc/fs_pd.h             |    9 ++++++---
 include/asm-ppc/fs_pd.h                 |    8 ++++++++
 7 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/sysdev/commproc.c b/arch/powerpc/sysdev/commproc.c
index 9b4fafd..44e63e8 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -39,15 +39,11 @@
 #include <asm/tlbflush.h>
 #include <asm/rheap.h>
 #include <asm/prom.h>
-
 #include <asm/fs_pd.h>
 
-#define CPM_MAP_SIZE    (0x4000)
-
 static void m8xx_cpm_dpinit(void);
 static	uint	host_buffer;	/* One page of host buffer */
 static	uint	host_end;	/* end + 1 */
-cpm8xx_t	*cpmp;		/* Pointer to comm processor space */
 cpic8xx_t	*cpic_reg;
 
 static struct device_node *cpm_pic_node;
@@ -200,7 +196,7 @@ void cpm_reset(void)
 	cpm8xx_t	*commproc;
 	sysconf8xx_t    *siu_conf;
 
-	commproc = (cpm8xx_t *)ioremap(CPM_MAP_ADDR, CPM_MAP_SIZE);
+	commproc = (cpm8xx_t *)immr_map(im_cpm);
 
 #ifdef CONFIG_UCODE_PATCH
 	/* Perform a reset.
@@ -213,6 +209,7 @@ void cpm_reset(void)
 
 	cpm_load_patch(commproc);
 #endif
+	immr_unmap(commproc);
 
 	/* Set SDMA Bus Request priority 5.
 	 * On 860T, this also enables FEC priority 6.  I am not sure
@@ -226,10 +223,6 @@ void cpm_reset(void)
 
 	/* Reclaim the DP memory for our use. */
 	m8xx_cpm_dpinit();
-
-	/* Tell everyone where the comm processor resides.
-	*/
-	cpmp = commproc;
 }
 
 /* We used to do this earlier, but have to postpone as long as possible
@@ -283,7 +276,7 @@ cpm_setbrg(uint brg, uint rate)
 
 	/* This is good enough to get SMCs running.....
 	*/
-	bp = (uint *)&cpmp->cp_brgc1;
+	bp = immr_map_size(im_cpm.cp_brgc1, 16);
 	bp += brg;
 	/* The BRG has a 12-bit counter.  For really slow baud rates (or
 	 * really fast processors), we may have to further divide by 16.
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index 5bd99c6..ddf4570 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -1131,13 +1131,8 @@ void *fs_enet_immap = NULL;
 
 static int setup_immap(void)
 {
-	phys_addr_t paddr = 0;
-	unsigned long size = 0;
-
 #ifdef CONFIG_CPM1
-	paddr = IMAP_ADDR;
-	size = 0x10000; /* map 64K */
-	fs_enet_immap = ioremap(paddr, size);
+	fs_enet_immap = immr_map(im_cpm);
 #endif
 
 #ifdef CONFIG_CPM2
diff --git a/drivers/net/fs_enet/mac-scc.c b/drivers/net/fs_enet/mac-scc.c
index 65925b5..775f92f 100644
--- a/drivers/net/fs_enet/mac-scc.c
+++ b/drivers/net/fs_enet/mac-scc.c
@@ -90,7 +90,7 @@
 
 static inline int scc_cr_cmd(struct fs_enet_private *fep, u32 op)
 {
-	cpm8xx_t *cpmp = &((immap_t *)fs_enet_immap)->im_cpm;
+	cpm8xx_t *cpmp = fs_enet_immap;
 	u32 v, ch;
 	int i = 0;
 
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
index 925fb60..64a52c5 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
@@ -52,7 +52,9 @@
 void cpm_line_cr_cmd(int line, int cmd)
 {
 	ushort val;
-	volatile cpm8xx_t *cp = cpmp;
+	volatile cpm8xx_t *cp;
+	
+	cp = (cpm8xx_t *)immr_map(im_cpm);
 
 	switch (line) {
 	case UART_SMC1:
@@ -187,6 +189,8 @@ void cpm_uart_freebuf(struct uart_cpm_port *pinfo)
 /* Setup any dynamic params in the uart desc */
 int __init cpm_uart_init_portdesc(void)
 {
+	cpm8xx_t *cpmp = (cpm8xx_t *)immr_map(im_cpm);
+
 	pr_debug("CPM uart[-]:init portdesc\n");
 
 	cpm_uart_nr = 0;
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.h b/drivers/serial/cpm_uart/cpm_uart_cpm1.h
index a99e45e..2a64778 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm1.h
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.h
@@ -37,6 +37,6 @@ static inline void cpm_set_smc_fcr(volatile smc_uart_t * up)
 	up->smc_tfcr = SMC_EB;
 }
 
-#define DPRAM_BASE	((unsigned char *)&cpmp->cp_dpmem[0])
+#define DPRAM_BASE	((unsigned char *)cpm_dpram_addr(0))
 
 #endif
diff --git a/include/asm-powerpc/fs_pd.h b/include/asm-powerpc/fs_pd.h
index 77a44f9..bc75442 100644
--- a/include/asm-powerpc/fs_pd.h
+++ b/include/asm-powerpc/fs_pd.h
@@ -37,12 +37,11 @@
 
 #ifdef CONFIG_8xx
 #include <asm/8xx_immap.h>
-#include <asm/mpc8xx.h>
 
 #define immr_map(member)						\
 ({									\
 	u32 offset = offsetof(immap_t, member);				\
-	void *addr = ioremap (IMAP_ADDR + offset,			\
+	void *addr = ioremap (get_immrbase() + offset,			\
 			      sizeof( ((immap_t*)0)->member));		\
 	addr;								\
 })
@@ -50,13 +49,17 @@
 #define immr_map_size(member, size)					\
 ({									\
 	u32 offset = offsetof(immap_t, member);				\
-	void *addr = ioremap (IMAP_ADDR + offset, size);		\
+	void *addr = ioremap (get_immrbase() + offset, size);		\
 	addr;								\
 })
 
 #define immr_unmap(addr)		iounmap(addr)
 #endif
 
+#ifndef CPM_IRQ_OFFSET
+#define CPM_IRQ_OFFSET	0	/* Backword-compatibility stuff for the drivers */
+#endif
+
 static inline int uart_baudrate(void)
 {
         return get_baudrate();
diff --git a/include/asm-ppc/fs_pd.h b/include/asm-ppc/fs_pd.h
index 8691327..b3ec063 100644
--- a/include/asm-ppc/fs_pd.h
+++ b/include/asm-ppc/fs_pd.h
@@ -29,8 +29,16 @@ static inline int uart_clock(void)
 	return (((bd_t *) __res)->bi_intfreq);
 }
 
+#ifdef CONFIG_CPM2
 #define cpm2_map(member)	(&cpm2_immr->member)
 #define cpm2_map_size(member, size)	(&cpm2_immr->member)
 #define cpm2_unmap(addr)        do {} while(0)
+#endif
+
+#ifdef CONFIG_8xx
+#define immr_map(member)	(&((immap_t *)IMAP_ADDR)->member)
+#define immr_map_size(member, size)	(&((immap_t *)IMAP_ADDR)->member)
+#define immr_unmap(addr)        do {} while(0)
+#endif
 
 #endif

Re: [PATCH 2/2] POWERPC: Remove global CPM mappings

[Dan Malek]

On Apr 23, 2007, at 6:39 PM, Vitaly Bordug wrote:

>
> Gets rid or direct IMMR accesses/dereferences for PQ SoC targets and
> relevant drivers.

Can we find a way to do this without the constant
ioremapping and unmapping?  It's a quick and
dirty hack, but it would be nice if we could map once
during an initialization, keep the pointer around
and at least use it within the file context.  I know it
doesn't cost much, but all of these cycles add up,
and someday ioremap could grow into something
more complex.  Please?

The whole of the CPM supporting functions need this,
it could be mapped once and shared at least among
them.  Any drivers that use the IMMR should also be
cleaner about managing this space as they would
any other mapped resource that is typically done
only once for the life of the driver.

There are also many different methods used that
I don't understand the reason.  Sometimes the
mappings are hidden in macros, sometimes done
in a function that is called, other times ioremap()
is called explicitly.  It just looks, well....  icky.  :-)


Thanks.

	-- Dan

Re: [PATCH 2/2] POWERPC: Remove global CPM mappings

[Vitaly Bordug]

On Mon, 23 Apr 2007 21:26:59 -0400
Dan Malek wrote:

> 
> On Apr 23, 2007, at 6:39 PM, Vitaly Bordug wrote:
> 
> >
> > Gets rid or direct IMMR accesses/dereferences for PQ SoC targets and
> > relevant drivers.
> 
> Can we find a way to do this without the constant
> ioremapping and unmapping?  It's a quick and
> dirty hack, but it would be nice if we could map once
> during an initialization, keep the pointer around
> and at least use it within the file context.  I know it
> doesn't cost much, but all of these cycles add up,
> and someday ioremap could grow into something
> more complex.  Please?

Well, I know the solution is not ideal, neither it is final,
but it is a step forward in cleaning up CPM-related stuff.
In many drivers it used to be just modifications of immr->foo, 
which fills driver with pleasant board-specific ifdefs and gives hard time 
to figure out with something's not working as expected.

At first I was thinking of make_everybody_happy solution, 
but with the whole kernel moving forward even brilliant ideas tend to 
stay at draft stage (well, you know that much better than me:) )
> 
> The whole of the CPM supporting functions need this,
> it could be mapped once and shared at least among
> them.  Any drivers that use the IMMR should also be
> cleaner about managing this space as they would
> any other mapped resource that is typically done
> only once for the life of the driver.
> 

I know it can be more efficient. And I am looking at this way, but it just cannot
be achieved via single step. TODO list includes rehaul of GPIO (with long-time-grown 
feature_call + device tree bindings that were implemented for 8360 but looks reasonable), muxing, etc.

Also I had to take care of arch/ppc since most affected cpm-related drivers (as well as targets) exist and 
work both at ppc/ and powerpc/. 
> There are also many different methods used that
> I don't understand the reason.  Sometimes the
> mappings are hidden in macros, sometimes done
> in a function that is called, other times ioremap()
> is called explicitly.  It just looks, well....  icky.  :-)
> 

This patch just fixes what already exist in kernel, removing the global IMMR pointer and bringing all remaining code paths to the same need_stuff->immr_map->use_it->immr_unmap model. IOW , wraps up what has began before. This ppc/powerpc scatter makes it pretty tricky to move drivers forward without ugly code to handle both 
or leaving one of the ways aside, and this approach gives sane code that works in both cases. Current code is messy at some parts, and this what I am trying to address (with current patch and upcomings).

Thanks for looking at it!

-Vitaly

Re: [PATCH 2/2] POWERPC: Remove global CPM mappings

[Dan Malek]

On Apr 24, 2007, at 1:18 AM, Vitaly Bordug wrote:

> At first I was thinking of make_everybody_happy solution,

Just make me happy :-)  The problem with these
"works in progress" submissions is no one ever
goes back to properly finish them, so I suspect
this code you are checking in, which I don't
like at all, is going to just stay there and bug
me until _I_ fix it.


> I know it can be more efficient. And I am looking at this way, but  
> it just cannot
> be achieved via single step.

Why?  Because it's more work than you want to do?

> TODO list includes rehaul of GPIO (with long-time-grown
> feature_call + device tree bindings that were implemented for 8360  
> but looks reasonable), muxing, etc.

Nothing you have done so far affects any of this.
The things you are changing are the very basic
support functions.  You'll never go back and
fix these because then the excuse will be
"all of the drivers now use it."

> This patch just fixes what already exist in kernel, removing the  
> global IMMR pointer

It doesn't "fix" anything, it just changes the model to make it
more heavyweight.

> and bringing all remaining code paths to the same need_stuff- 
> >immr_map->use_it->immr_unmap model.

That's not the model.  The model is:  driver init maps IMMR,
driver uses it's local mapping, driver unloaded unmaps IMMR.
We don't map/unmap on every use.  The macros should be
given this mapped immr as a parameter, not always
map and unmap because it takes more than five minutes
to properly change all of the APIs.

> Current code is messy at some parts, and this what I am trying to  
> address (with current patch and upcomings).

IMHO this does nothing to clean it up.  I don't understand
the big deal about needing to remove the IMMR as a global
pointer.  The kernel has lots of other global variables that
no one seems concerned about and admit they must be
present.  These crappy macros and hacks in the name of
removing a global IMMR just isn't right.

> Thanks for looking at it!

I'm not in any agreement this is correct nor do I want to
see the crap checked in.  You're turning a simple,
couple of clock cycle memory access into huge overhead,
probably context switchable operation.  Someone is
going to grab a spin lock, and end up crashing on
what appears to be a trivial memory access.....

This is not progress.


Thanks.

	-- Dan

Re: [PATCH 2/2] POWERPC: Remove global CPM mappings

[Vitaly Bordug]

On Tue, 24 Apr 2007 16:24:42 -0400
Dan Malek wrote:

> 
> On Apr 24, 2007, at 1:18 AM, Vitaly Bordug wrote:
> 
> > At first I was thinking of make_everybody_happy solution,
> 
> Just make me happy :-)  The problem with these
> "works in progress" submissions is no one ever
> goes back to properly finish them, so I suspect
> this code you are checking in, which I don't
> like at all, is going to just stay there and bug
> me until _I_ fix it.
> 
Hmm - it seems I want to check stuff in and forget about it? That's just not right, and there is zero value in it.
> 
> > I know it can be more efficient. And I am looking at this way, but  
> > it just cannot
> > be achieved via single step.
> 
> Why?  Because it's more work than you want to do?
> 
Because one_step_for_all is breakage-prone; and small changes are easier to debug. I want to make CPM layer
stuff better while keeping it working, and definitely not just to do some specific amount of work/code, sorry.

> > TODO list includes rehaul of GPIO (with long-time-grown
> > feature_call + device tree bindings that were implemented for 8360  
> > but looks reasonable), muxing, etc.
> 
> Nothing you have done so far affects any of this.
> The things you are changing are the very basic
> support functions.  You'll never go back and
> fix these because then the excuse will be
> "all of the drivers now use it."
> 
It does not matter what "all of the drivers now use". I just dumped long-term plan.

Anyway let's refrain from predicting future - this is kinda off topic here ;).



> > This patch just fixes what already exist in kernel, removing the  
> > global IMMR pointer
> 
> It doesn't "fix" anything, it just changes the model to make it
> more heavyweight.
> 
This approach assumes keeping whole mapped immr just to toggle a bit in mux or issue cpm_cr_cmd
is not a good thing. If the driver needs to initialize values in mux(once), it makes sense to remap that part of immr and do just that. 

for "It doesn't "fix" anything", this particular patch wraps up the cleanup and does not actually introduce new
model. The only critial path I can admit is CPM command stuff inside fs_enet and uart drivers, I'll fix it if that is the point.

> > and bringing all remaining code paths to the same need_stuff- 
> > >immr_map->use_it->immr_unmap model.
> 
> That's not the model.  The model is:  driver init maps IMMR,
> driver uses it's local mapping, driver unloaded unmaps IMMR.
> We don't map/unmap on every use.  The macros should be
> given this mapped immr as a parameter, not always
> map and unmap because it takes more than five minutes
> to properly change all of the APIs.
> 

So you see good in each CPM-related driver keeping its own remapped immr? It's up to driver/implementation how to handle stuff: it can remap needed things upon init and keep offsets around in internal struct, or have it ioremapped and modified in case of single-shot init/reconfigure thing. Macro is just a tool to access only necessary part of immr, and is kind of hacky just to keep arch/ppc in the loop.

> > Current code is messy at some parts, and this what I am trying to  
> > address (with current patch and upcomings).
> 
> IMHO this does nothing to clean it up.  I don't understand
> the big deal about needing to remove the IMMR as a global
> pointer.  The kernel has lots of other global variables that
> no one seems concerned about and admit they must be
> present.  These crappy macros and hacks in the name of
> removing a global IMMR just isn't right.
> 

Again, macros is used to keep arch/ppc/ working. direct immr usage may be efficient but pretty hacky too.
The patch is changing only arch/powerpc stuff and is sort of formal because does not really affect anything(in arch/powerpc of course) except cpm_reset and brg setting.

> > Thanks for looking at it!
> 
> I'm not in any agreement this is correct nor do I want to
> see the crap checked in.  You're turning a simple,
> couple of clock cycle memory access into huge overhead,
> probably context switchable operation.  Someone is
> going to grab a spin lock, and end up crashing on
> what appears to be a trivial memory access.....
> 
> This is not progress.

Erm.. Where exactly in this patch - cr_cmd stuff in drivers/? Overhead relies on implementation - nobody would be happy of the incremental ioremapping. 

-V

[PATCH 1/2] POWERPC: Remove global CPM2 mappings

[Vitaly Bordug]

Gets rid or direct IMMR accesses/dereferences for PQ2 SoC targets and
relevant drivers. 

Previous approach was to setup the specific immr offset early and make whole immr structure open via global variable/define. Now in order to access some specific immr part it has to be mapped. Along with the clarity, it deprecates many board-specific ifdefs from common drivers code as well as platform header files.

Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>
---

 arch/powerpc/platforms/82xx/mpc82xx_ads.c |    6 +++-
 arch/powerpc/platforms/82xx/pq2ads.h      |    4 ---
 arch/powerpc/sysdev/cpm2_common.c         |   17 +-----------
 arch/powerpc/sysdev/fsl_soc.c             |   40 +++++++++++++++++++++++++----
 arch/powerpc/sysdev/fsl_soc.h             |    1 +
 drivers/net/fs_enet/fs_enet-main.c        |    8 +++---
 drivers/net/fs_enet/mac-fcc.c             |    3 +-
 drivers/serial/cpm_uart/cpm_uart_cpm2.h   |   12 ++++-----
 include/asm-powerpc/fs_pd.h               |   14 ++++------
 9 files changed, 57 insertions(+), 48 deletions(-)

diff --git a/arch/powerpc/platforms/82xx/mpc82xx_ads.c b/arch/powerpc/platforms/82xx/mpc82xx_ads.c
index 7334c1a..f2fd07c 100644
--- a/arch/powerpc/platforms/82xx/mpc82xx_ads.c
+++ b/arch/powerpc/platforms/82xx/mpc82xx_ads.c
@@ -46,6 +46,7 @@
 #include <asm/cpm2.h>
 #include <asm/udbg.h>
 #include <asm/i8259.h>
+#include <asm/fs_pd.h>
 #include <linux/fs_enet_pd.h>
 
 #include <sysdev/fsl_soc.h>
@@ -615,13 +616,14 @@ static int __init mpc82xx_ads_probe(void)
 static void m82xx_restart(char *cmd)
 {
 	__volatile__ unsigned char dummy;
+	car_cpm2_t *clk = cpm2_map(im_clkrst);
 
 	local_irq_disable();
-	((cpm2_map_t *) cpm2_immr)->im_clkrst.car_rmr |= RMR_CSRE;
+	setbits32(&clk->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];
+	dummy = in_8(&clk->res[0]);
 	printk("Restart failed\n");
 	while (1) ;
 }
diff --git a/arch/powerpc/platforms/82xx/pq2ads.h b/arch/powerpc/platforms/82xx/pq2ads.h
index 5b5cca6..f0cd179 100644
--- a/arch/powerpc/platforms/82xx/pq2ads.h
+++ b/arch/powerpc/platforms/82xx/pq2ads.h
@@ -29,10 +29,6 @@
 #define CPUINFO_VENDOR		"Freescale Semiconductor"
 #define CPUINFO_MACHINE		"PQ2 ADS PowerPC"
 
-/* Backword-compatibility stuff for the drivers */
-#define CPM_MAP_ADDR		((uint)0xf0000000)
-#define CPM_IRQ_OFFSET 0
-
 /* The ADS8260 has 16, 32-bit wide control/status registers, accessed
  * only on word boundaries.
  * Not all are used (yet), or are interesting to us (yet).
diff --git a/arch/powerpc/sysdev/cpm2_common.c b/arch/powerpc/sysdev/cpm2_common.c
index ec26599..229a4c0 100644
--- a/arch/powerpc/sysdev/cpm2_common.c
+++ b/arch/powerpc/sysdev/cpm2_common.c
@@ -45,31 +45,16 @@
 #include <sysdev/fsl_soc.h>
 
 static void cpm2_dpinit(void);
-cpm_cpm2_t	*cpmp;		/* Pointer to comm processor space */
-
-/* We allocate this here because it is used almost exclusively for
- * the communication processor devices.
- */
-cpm2_map_t *cpm2_immr;
 intctl_cpm2_t *cpm2_intctl;
 
-#define CPM_MAP_SIZE	(0x40000)	/* 256k - the PQ3 reserve this amount
-					   of space for CPM as it is larger
-					   than on PQ2 */
-
 void
 cpm2_reset(void)
 {
-	cpm2_immr = (cpm2_map_t *)ioremap(CPM_MAP_ADDR, CPM_MAP_SIZE);
 	cpm2_intctl = cpm2_map(im_intctl);
 
 	/* Reclaim the DP memory for our use.
 	 */
 	cpm2_dpinit();
-
-	/* Tell everyone where the comm processor resides.
-	 */
-	cpmp = &cpm2_immr->im_cpm;
 }
 
 /* Set a baud rate generator.  This needs lots of work.  There are
@@ -234,7 +219,7 @@ static void cpm2_dpinit(void)
 {
 	spin_lock_init(&cpm_dpmem_lock);
 
-	im_dprambase = ioremap(CPM_MAP_ADDR, CPM_DATAONLY_BASE + CPM_DATAONLY_SIZE);
+	im_dprambase = ioremap(get_cpmbase(), FCC_MEM_OFFSET(4));
 
 	/* initialize the info header */
 	rh_init(&cpm_dpmem_info, 1,
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index d20f029..ba7cd6f 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -35,7 +35,7 @@
 #include <asm/prom.h>
 #include <sysdev/fsl_soc.h>
 #include <mm/mmu_decl.h>
-#include <asm/cpm2.h>
+#include <asm/fs_pd.h>
 
 extern void init_fcc_ioports(struct fs_platform_info*);
 extern void init_fec_ioports(struct fs_platform_info*);
@@ -66,6 +66,30 @@ EXPORT_SYMBOL(get_immrbase);
 
 #if defined(CONFIG_CPM2) || defined(CONFIG_8xx)
 
+static phys_addr_t cpmbase = -1;
+
+phys_addr_t get_cpmbase(void)
+{
+	struct device_node *cpm;
+
+	if (cpmbase != -1)
+		return cpmbase;
+
+	cpm = of_find_node_by_type(NULL, "cpm");
+	if (cpm) {
+		unsigned int size;
+		const void *prop = get_property(cpm, "reg", &size);
+
+		if (prop)
+			cpmbase = of_translate_address(cpm, prop);
+		of_node_put(cpm);
+	};
+
+	return cpmbase;
+}
+
+EXPORT_SYMBOL(get_cpmbase);
+
 static u32 brgfreq = -1;
 
 u32 get_brgfreq(void)
@@ -115,6 +139,7 @@ u32 get_baudrate(void)
 }
 
 EXPORT_SYMBOL(get_baudrate);
+
 #endif /* CONFIG_CPM2 */
 
 static int __init gfar_mdio_of_init(void)
@@ -592,6 +617,7 @@ static int __init fs_enet_of_init(void)
 	struct platform_device *fs_enet_dev;
 	struct resource res;
 	int ret;
+	int configured = 0;
 
 	for (np = NULL, i = 0;
 	     (np = of_find_compatible_node(np, "network", "fs_enet")) != NULL;
@@ -691,13 +717,14 @@ static int __init fs_enet_of_init(void)
 			fs_enet_data.init_ioports = init_fcc_ioports;
 
 			mdio_bb_prop = get_property(phy, "bitbang", NULL);
-			if (mdio_bb_prop) {
+			if (mdio_bb_prop && !configured) {
 				struct platform_device *fs_enet_mdio_bb_dev;
 				struct fs_mii_bb_platform_info fs_enet_mdio_bb_data;
+				struct io_port *io = cpm2_map(im_ioport);
 
 				fs_enet_mdio_bb_dev =
 					platform_device_register_simple("fsl-bb-mdio",
-							i, NULL, 0);
+							res.start, NULL, 0);
 				memset(&fs_enet_mdio_bb_data, 0,
 						sizeof(struct fs_mii_bb_platform_info));
 				fs_enet_mdio_bb_data.mdio_dat.bit =
@@ -720,11 +747,11 @@ static int __init fs_enet_of_init(void)
 				fs_enet_mdio_bb_data.irq[31] = -1;
 
 				fs_enet_mdio_bb_data.mdio_dat.offset =
-					(u32)&cpm2_immr->im_ioport.iop_pdatc;
+					(u32)&io->iop_pdatc;
 				fs_enet_mdio_bb_data.mdio_dir.offset =
-					(u32)&cpm2_immr->im_ioport.iop_pdirc;
+					(u32)&io->iop_pdirc;
 				fs_enet_mdio_bb_data.mdc_dat.offset =
-					(u32)&cpm2_immr->im_ioport.iop_pdatc;
+					(u32)&io->iop_pdatc;
 
 				ret = platform_device_add_data(
 						fs_enet_mdio_bb_dev,
@@ -732,6 +759,7 @@ static int __init fs_enet_of_init(void)
 						sizeof(struct fs_mii_bb_platform_info));
 				if (ret)
 					goto unreg;
+				configured++;
 			}
 
 			of_node_put(phy);
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 04e145b..debcfdb 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -5,6 +5,7 @@
 #include <asm/mmu.h>
 
 extern phys_addr_t get_immrbase(void);
+extern phys_addr_t get_cpmbase(void);
 extern u32 get_brgfreq(void);
 extern u32 get_baudrate(void);
 
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index 4a05c14..5bd99c6 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -1136,14 +1136,14 @@ static int setup_immap(void)
 
 #ifdef CONFIG_CPM1
 	paddr = IMAP_ADDR;
-	size = 0x10000;	/* map 64K */
+	size = 0x10000; /* map 64K */
+	fs_enet_immap = ioremap(paddr, size);
 #endif
 
 #ifdef CONFIG_CPM2
-	paddr = CPM_MAP_ADDR;
-	size = 0x40000;	/* map 256 K */
+	fs_enet_immap = cpm2_map(im_cpm);
 #endif
-	fs_enet_immap = ioremap(paddr, size);
+
 	if (fs_enet_immap == NULL)
 		return -EBADF;	/* XXX ahem; maybe just BUG_ON? */
 
diff --git a/drivers/net/fs_enet/mac-fcc.c b/drivers/net/fs_enet/mac-fcc.c
index 8545e84..bd803ba 100644
--- a/drivers/net/fs_enet/mac-fcc.c
+++ b/drivers/net/fs_enet/mac-fcc.c
@@ -88,8 +88,7 @@ static inline int fcc_cr_cmd(struct fs_enet_private *fep, u32 mcn, u32 op)
 {
 	const struct fs_platform_info *fpi = fep->fpi;
 
-	cpm2_map_t *immap = fs_enet_immap;
-	cpm_cpm2_t *cpmp = &immap->im_cpm;
+	cpm_cpm2_t *cpmp = fs_enet_immap;
 	u32 v;
 	int i;
 
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.h b/drivers/serial/cpm_uart/cpm_uart_cpm2.h
index 1b3219f..34978fc 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm2.h
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.h
@@ -13,12 +13,12 @@
 #include <asm/cpm2.h>
 
 /* defines for IRQs */
-#define SMC1_IRQ	SIU_INT_SMC1
-#define SMC2_IRQ	SIU_INT_SMC2
-#define SCC1_IRQ	SIU_INT_SCC1
-#define SCC2_IRQ	SIU_INT_SCC2
-#define SCC3_IRQ	SIU_INT_SCC3
-#define SCC4_IRQ	SIU_INT_SCC4
+#define SMC1_IRQ	(CPM_IRQ_OFFSET + 0x04)
+#define SMC2_IRQ	(CPM_IRQ_OFFSET + 0x05)
+#define SCC1_IRQ	(CPM_IRQ_OFFSET + 0x28)
+#define SCC2_IRQ	(CPM_IRQ_OFFSET + 0x29)
+#define SCC3_IRQ	(CPM_IRQ_OFFSET + 0x2a)
+#define SCC4_IRQ	(CPM_IRQ_OFFSET + 0x2b)
 
 static inline void cpm_set_brg(int brg, int baud)
 {
diff --git a/include/asm-powerpc/fs_pd.h b/include/asm-powerpc/fs_pd.h
index c624915..77a44f9 100644
--- a/include/asm-powerpc/fs_pd.h
+++ b/include/asm-powerpc/fs_pd.h
@@ -17,16 +17,10 @@
 #ifdef CONFIG_CPM2
 #include <asm/cpm2.h>
 
-#if defined(CONFIG_8260)
-#include <asm/mpc8260.h>
-#elif defined(CONFIG_85xx)
-#include <asm/mpc85xx.h>
-#endif
-
 #define cpm2_map(member)						\
 ({									\
 	u32 offset = offsetof(cpm2_map_t, member);			\
-	void *addr = ioremap (CPM_MAP_ADDR + offset,			\
+	void *addr = ioremap (get_cpmbase() + offset,			\
 			      sizeof( ((cpm2_map_t*)0)->member));	\
 	addr;								\
 })
@@ -34,7 +28,7 @@
 #define cpm2_map_size(member, size)					\
 ({									\
 	u32 offset = offsetof(cpm2_map_t, member);			\
-	void *addr = ioremap (CPM_MAP_ADDR + offset, size);		\
+	void *addr = ioremap (get_cpmbase() + offset,			\
 	addr;								\
 })
 
@@ -73,4 +67,8 @@ static inline int uart_clock(void)
         return ppc_proc_freq;
 }
 
+#ifndef CPM_IRQ_OFFSET
+#define CPM_IRQ_OFFSET 0       /* Backword-compatibility stuff for the drivers */
+#endif
+
 #endif

[PATCH 2/2] POWERPC: Remove global CPM mappings

[Vitaly Bordug]

Gets rid or direct IMMR accesses/dereferences for PQ SoC targets and
relevant drivers.

Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>
---

 arch/powerpc/sysdev/commproc.c          |   13 +++----------
 drivers/net/fs_enet/fs_enet-main.c      |    7 +------
 drivers/net/fs_enet/mac-scc.c           |    2 +-
 drivers/serial/cpm_uart/cpm_uart_cpm1.c |    4 +++-
 drivers/serial/cpm_uart/cpm_uart_cpm1.h |    2 +-
 include/asm-powerpc/fs_pd.h             |    9 ++++++---
 include/asm-ppc/fs_pd.h                 |    8 ++++++++
 7 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/sysdev/commproc.c b/arch/powerpc/sysdev/commproc.c
index 9b4fafd..44e63e8 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -39,15 +39,11 @@
 #include <asm/tlbflush.h>
 #include <asm/rheap.h>
 #include <asm/prom.h>
-
 #include <asm/fs_pd.h>
 
-#define CPM_MAP_SIZE    (0x4000)
-
 static void m8xx_cpm_dpinit(void);
 static	uint	host_buffer;	/* One page of host buffer */
 static	uint	host_end;	/* end + 1 */
-cpm8xx_t	*cpmp;		/* Pointer to comm processor space */
 cpic8xx_t	*cpic_reg;
 
 static struct device_node *cpm_pic_node;
@@ -200,7 +196,7 @@ void cpm_reset(void)
 	cpm8xx_t	*commproc;
 	sysconf8xx_t    *siu_conf;
 
-	commproc = (cpm8xx_t *)ioremap(CPM_MAP_ADDR, CPM_MAP_SIZE);
+	commproc = (cpm8xx_t *)immr_map(im_cpm);
 
 #ifdef CONFIG_UCODE_PATCH
 	/* Perform a reset.
@@ -213,6 +209,7 @@ void cpm_reset(void)
 
 	cpm_load_patch(commproc);
 #endif
+	immr_unmap(commproc);
 
 	/* Set SDMA Bus Request priority 5.
 	 * On 860T, this also enables FEC priority 6.  I am not sure
@@ -226,10 +223,6 @@ void cpm_reset(void)
 
 	/* Reclaim the DP memory for our use. */
 	m8xx_cpm_dpinit();
-
-	/* Tell everyone where the comm processor resides.
-	*/
-	cpmp = commproc;
 }
 
 /* We used to do this earlier, but have to postpone as long as possible
@@ -283,7 +276,7 @@ cpm_setbrg(uint brg, uint rate)
 
 	/* This is good enough to get SMCs running.....
 	*/
-	bp = (uint *)&cpmp->cp_brgc1;
+	bp = immr_map_size(im_cpm.cp_brgc1, 16);
 	bp += brg;
 	/* The BRG has a 12-bit counter.  For really slow baud rates (or
 	 * really fast processors), we may have to further divide by 16.
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index 5bd99c6..ddf4570 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -1131,13 +1131,8 @@ void *fs_enet_immap = NULL;
 
 static int setup_immap(void)
 {
-	phys_addr_t paddr = 0;
-	unsigned long size = 0;
-
 #ifdef CONFIG_CPM1
-	paddr = IMAP_ADDR;
-	size = 0x10000; /* map 64K */
-	fs_enet_immap = ioremap(paddr, size);
+	fs_enet_immap = immr_map(im_cpm);
 #endif
 
 #ifdef CONFIG_CPM2
diff --git a/drivers/net/fs_enet/mac-scc.c b/drivers/net/fs_enet/mac-scc.c
index 65925b5..775f92f 100644
--- a/drivers/net/fs_enet/mac-scc.c
+++ b/drivers/net/fs_enet/mac-scc.c
@@ -90,7 +90,7 @@
 
 static inline int scc_cr_cmd(struct fs_enet_private *fep, u32 op)
 {
-	cpm8xx_t *cpmp = &((immap_t *)fs_enet_immap)->im_cpm;
+	cpm8xx_t *cpmp = fs_enet_immap;
 	u32 v, ch;
 	int i = 0;
 
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
index 925fb60..d09ab98 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
@@ -52,7 +52,7 @@
 void cpm_line_cr_cmd(int line, int cmd)
 {
 	ushort val;
-	volatile cpm8xx_t *cp = cpmp;
+	volatile cpm8xx_t *cp = (cpm8xx_t *)immr_map(im_cpm);
 
 	switch (line) {
 	case UART_SMC1:
@@ -187,6 +187,8 @@ void cpm_uart_freebuf(struct uart_cpm_port *pinfo)
 /* Setup any dynamic params in the uart desc */
 int __init cpm_uart_init_portdesc(void)
 {
+	cpm8xx_t *cpmp = (cpm8xx_t *)immr_map(im_cpm);
+
 	pr_debug("CPM uart[-]:init portdesc\n");
 
 	cpm_uart_nr = 0;
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.h b/drivers/serial/cpm_uart/cpm_uart_cpm1.h
index a99e45e..2a64778 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm1.h
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.h
@@ -37,6 +37,6 @@ static inline void cpm_set_smc_fcr(volatile smc_uart_t * up)
 	up->smc_tfcr = SMC_EB;
 }
 
-#define DPRAM_BASE	((unsigned char *)&cpmp->cp_dpmem[0])
+#define DPRAM_BASE	((unsigned char *)cpm_dpram_addr(0))
 
 #endif
diff --git a/include/asm-powerpc/fs_pd.h b/include/asm-powerpc/fs_pd.h
index 77a44f9..bc75442 100644
--- a/include/asm-powerpc/fs_pd.h
+++ b/include/asm-powerpc/fs_pd.h
@@ -37,12 +37,11 @@
 
 #ifdef CONFIG_8xx
 #include <asm/8xx_immap.h>
-#include <asm/mpc8xx.h>
 
 #define immr_map(member)						\
 ({									\
 	u32 offset = offsetof(immap_t, member);				\
-	void *addr = ioremap (IMAP_ADDR + offset,			\
+	void *addr = ioremap (get_immrbase() + offset,			\
 			      sizeof( ((immap_t*)0)->member));		\
 	addr;								\
 })
@@ -50,13 +49,17 @@
 #define immr_map_size(member, size)					\
 ({									\
 	u32 offset = offsetof(immap_t, member);				\
-	void *addr = ioremap (IMAP_ADDR + offset, size);		\
+	void *addr = ioremap (get_immrbase() + offset, size);		\
 	addr;								\
 })
 
 #define immr_unmap(addr)		iounmap(addr)
 #endif
 
+#ifndef CPM_IRQ_OFFSET
+#define CPM_IRQ_OFFSET	0	/* Backword-compatibility stuff for the drivers */
+#endif
+
 static inline int uart_baudrate(void)
 {
         return get_baudrate();
diff --git a/include/asm-ppc/fs_pd.h b/include/asm-ppc/fs_pd.h
index 8691327..b3ec063 100644
--- a/include/asm-ppc/fs_pd.h
+++ b/include/asm-ppc/fs_pd.h
@@ -29,8 +29,16 @@ static inline int uart_clock(void)
 	return (((bd_t *) __res)->bi_intfreq);
 }
 
+#ifdef CONFIG_CPM2
 #define cpm2_map(member)	(&cpm2_immr->member)
 #define cpm2_map_size(member, size)	(&cpm2_immr->member)
 #define cpm2_unmap(addr)        do {} while(0)
+#endif
+
+#ifdef CONFIG_8xx
+#define immr_map(member)	(&((immap_t *)IMAP_ADDR)->member)
+#define immr_map_size(member, size)	(&((immap_t *)IMAP_ADDR)->member)
+#define immr_unmap(addr)        do {} while(0)
+#endif
 
 #endif

Re: [PATCH 2/2] POWERPC: Remove global CPM mappings

[Dan Malek]

On Mar 27, 2007, at 5:01 PM, Vitaly Bordug wrote:

> --- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> @@ -52,7 +52,7 @@
>  void cpm_line_cr_cmd(int line, int cmd)
>  {
>  	ushort val;
> -	volatile cpm8xx_t *cp = cpmp;
> +	volatile cpm8xx_t *cp = (cpm8xx_t *)immr_map(im_cpm);

I'd kind of prefer you didn't place function calls
or macros up here with the variable declarations.
Do the initialization down in the code section, please.

> +#ifdef CONFIG_CPM2
>  #define cpm2_map(member)	(&cpm2_immr->member)
>  #define cpm2_map_size(member, size)	(&cpm2_immr->member)
>  #define cpm2_unmap(addr)        do {} while(0)
> +#endif
> +
> +#ifdef CONFIG_8xx
> +#define immr_map(member)	(&((immap_t *)IMAP_ADDR)->member)
> +#define immr_map_size(member, size)	(&((immap_t *)IMAP_ADDR)->member)
> +#define immr_unmap(addr)        do {} while(0)
> +#endif

Why is 8xx different?  We should be able to
devine cpm_immr and use it in both cases here,
and hopefully use the same macro names
across all drivers, too.


Thanks.

	-- Dan

Re: [PATCH 2/2] POWERPC: Remove global CPM mappings

[Vitaly Bordug]

On Tue, 27 Mar 2007 18:12:38 -0400
Dan Malek wrote:

> 
> On Mar 27, 2007, at 5:01 PM, Vitaly Bordug wrote:
> 
> > --- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> > +++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> > @@ -52,7 +52,7 @@
> >  void cpm_line_cr_cmd(int line, int cmd)
> >  {
> >  	ushort val;
> > -	volatile cpm8xx_t *cp = cpmp;
> > +	volatile cpm8xx_t *cp = (cpm8xx_t *)immr_map(im_cpm);
> 
> I'd kind of prefer you didn't place function calls
> or macros up here with the variable declarations.
> Do the initialization down in the code section, please.
> 

ok.
> > +#ifdef CONFIG_CPM2
> >  #define cpm2_map(member)	(&cpm2_immr->member)
> >  #define cpm2_map_size(member, size)	(&cpm2_immr->member)
> >  #define cpm2_unmap(addr)        do {} while(0)
> > +#endif
> > +
> > +#ifdef CONFIG_8xx
> > +#define immr_map(member)	(&((immap_t *)IMAP_ADDR)->member)
> > +#define immr_map_size(member, size)	(&((immap_t
> > *)IMAP_ADDR)->member) +#define immr_unmap(addr)        do {}
> > while(0) +#endif
> 
> Why is 8xx different?  We should be able to
> devine cpm_immr and use it in both cases here,
> and hopefully use the same macro names
> across all drivers, too.
> 
> 
The upper relates to arch/ppc code, which uses different structures and variables (cpm2_immr and immap respectively).  With this, I don't want to unify  cpm and cpm2 immrs, at least, not now. They are separated intentionally: while doing cpm2_map one gets access to one of the cpm2_map_t member, and doing immr_map, to immap_t member. 

Those 2 as you know better than anyone here are very similar and very different at the same time, and masking the difference is confusion-prone I think.

Thanks for looking into this!

--
Sincerely, Vitaly