powerpc: Move CPM command handling into the cpm drivers
Subsystems (2)
This patch moves the CPM command handling into commproc.c for CPM1 and cpm2_common.c. This is yet another preparation to get rid of drivers accessing the CPM via the global cpmp.
--- arch/powerpc/sysdev/commproc.c | 20 ++++++++++++++++++++ arch/powerpc/sysdev/cpm2_common.c | 17 +++++++++++++++++ drivers/net/fs_enet/mac-fcc.c | 10 +--------- drivers/net/fs_enet/mac-scc.c | 11 +---------- drivers/serial/cpm_uart/cpm_uart_cpm1.c | 6 +----- drivers/serial/cpm_uart/cpm_uart_cpm2.c | 8 +------- include/asm-powerpc/cpm.h | 1 + 7 files changed, 42 insertions(+), 31 deletions(-)
arch/powerpc/sysdev/commproc.c +20 -0
@@ -240,6 +240,26 @@ void __init cpm_reset(void)#endif } +#define MAX_CR_CMD_LOOPS 10000 + +int cpm_command (u32 command,u8 opcode) +{ +int i; + +if (command &0xffffff0f ) +return -EINVAL ; + +out_be16 (&cpmp->cp_cpcr, command |CPM_CR_FLG | (opcode <<8 )); +for (i =0 ; i <MAX_CR_CMD_LOOPS ; i++) +if ((in_be16 (&cpmp->cp_cpcr) &CPM_CR_FLG ) ==0 ) +return 0 ; + +printk (KERN_ERR "%s(): Not able to issue CPM command\n" , + __FUNCTION__); +return -EIO ; +} +EXPORT_SYMBOL (cpm_command); +/* We used to do this earlier, but have to postpone as long as possible * to ensure the kernel VM is now running. */
arch/powerpc/sysdev/cpm2_common.c +17 -0
@@ -83,6 +83,23 @@ cpm2_reset(void) cpmp = &cpm2_immr->im_cpm; } +#define MAX_CR_CMD_LOOPS 10000 + +int cpm_command (u32 command,u8 opcode) +{ +int i; + +out_be32 (&cpmp->cp_cpcr, command | opcode |CPM_CR_FLG ); +for (i =0 ; i <MAX_CR_CMD_LOOPS ; i++) +if ((in_be32 (&cpmp->cp_cpcr) &CPM_CR_FLG ) ==0 ) +return 0 ; + +printk (KERN_ERR "%s(): Not able to issue CPM command\n" , + __FUNCTION__); +return -EIO ; +} +EXPORT_SYMBOL (cpm_command); +/* Set a baud rate generator. This needs lots of work. There are * eight BRGs, which can be connected to the CPM channels or output * as clocks. The BRGs are in two different block of internal
drivers/net/fs_enet/mac-fcc.c +1 -9
@@ -81,16 +81,8 @@static inline int fcc_cr_cmd (struct fs_enet_private *fep,u32 op) {const struct fs_platform_info *fpi = fep->fpi; -int i; - -W32 (cpmp, cp_cpcr, fpi->cp_command | op |CPM_CR_FLG ); -for (i =0 ; i <MAX_CR_CMD_LOOPS ; i++) -if ((R32 (cpmp, cp_cpcr) &CPM_CR_FLG ) ==0 ) -return 0 ; -printk (KERN_ERR "%s(): Not able to issue CPM command\n" , - __FUNCTION__); -return 1 ; +return cpm_command (fpi->cp_command, op); }static int do_pd_setup (struct fs_enet_private *fep)
drivers/net/fs_enet/mac-scc.c +1 -10
@@ -89,21 +89,12 @@* Delay to wait for SCC reset command to complete (in us) */ #define SCC_RESET_DELAY 50 -#define MAX_CR_CMD_LOOPS 10000static inline int scc_cr_cmd (struct fs_enet_private *fep,u32 op) {const struct fs_platform_info *fpi = fep->fpi; -int i; - -W16 (cpmp, cp_cpcr, fpi->cp_command |CPM_CR_FLG | (op <<8 )); -for (i =0 ; i <MAX_CR_CMD_LOOPS ; i++) -if ((R16 (cpmp, cp_cpcr) &CPM_CR_FLG ) ==0 ) -return 0 ; -printk (KERN_ERR "%s(): Not able to issue CPM command\n" , - __FUNCTION__); -return 1 ; +return cpm_command (fpi->cp_command, op); }static int do_pd_setup (struct fs_enet_private *fep)
drivers/serial/cpm_uart/cpm_uart_cpm1.c +1 -5
@@ -52,11 +52,7 @@#ifdef CONFIG_PPC_CPM_NEW_BINDING void cpm_line_cr_cmd (struct uart_cpm_port *port,int cmd) { -u16 __iomem *cpcr = &cpmp->cp_cpcr; - -out_be16 (cpcr, port->command | (cmd <<8 ) |CPM_CR_FLG ); -while (in_be16 (cpcr) &CPM_CR_FLG ) - ; +cpm_command (port->command, cmd); }#else void cpm_line_cr_cmd (struct uart_cpm_port *port,int cmd)
drivers/serial/cpm_uart/cpm_uart_cpm2.c +1 -7
@@ -52,13 +52,7 @@#ifdef CONFIG_PPC_CPM_NEW_BINDING void cpm_line_cr_cmd (struct uart_cpm_port *port,int cmd) { -cpm_cpm2_t __iomem *cp =cpm2_map (im_cpm); - -out_be32 (&cp->cp_cpcr, port->command | cmd |CPM_CR_FLG ); -while (in_be32 (&cp->cp_cpcr) &CPM_CR_FLG ) - ; - -cpm2_unmap (cp); +cpm_command (port->command, cmd); }#else void cpm_line_cr_cmd (struct uart_cpm_port *port,int cmd)
include/asm-powerpc/cpm.h +1 -0
@@ -10,5 +10,6 @@ int cpm_muram_free(unsigned long offset);unsigned long cpm_muram_alloc_fixed (unsigned long offset,unsigned long size);void __iomem *cpm_muram_addr (unsigned long offset);dma_addr_t cpm_muram_dma (void __iomem *addr); +int cpm_command (u32 command,u8 opcode);#endif
Signature · 1 line
1.5.3.4
Quoted 30 lines · Jochen Friedrich
This patch moves the CPM command handling into
commproc.cfor CPM1 andcpm2_common.c. This is yet another preparation to get rid of drivers accessing the CPM via the global cpmp.Signed-off-by Jochen Friedrich <[email protected]>--- arch/powerpc/sysdev/commproc.c | 20 ++++++++++++++++++++ arch/powerpc/sysdev/cpm2_common.c | 17 +++++++++++++++++ drivers/net/fs_enet/mac-fcc.c | 10 +--------- drivers/net/fs_enet/mac-scc.c | 11 +---------- drivers/serial/cpm_uart/cpm_uart_cpm1.c | 6 +----- drivers/serial/cpm_uart/cpm_uart_cpm2.c | 8 +------- include/asm-powerpc/cpm.h | 1 + 7 files changed, 42 insertions(+), 31 deletions(-)arch/powerpc/sysdev/commproc.c +7
-0@@ -240,6 +240,26 @@ void __init cpm_reset(void)#endif } +#define MAX_CR_CMD_LOOPS 10000 + +int cpm_command (u32 command,u8 opcode) +{ +int i; + +if (command &0xffffff0f )
arch/powerpc/sysdev/commproc.c void __init cpm_reset(void) · line 259
Quoted 10 lines
arch/powerpc/sysdev/commproc.c +10
-0+return -EINVAL ; + +out_be16 (&cpmp->cp_cpcr, command |CPM_CR_FLG | (opcode <<8 )); +for (i =0 ; i <MAX_CR_CMD_LOOPS ; i++) +if ((in_be16 (&cpmp->cp_cpcr) &CPM_CR_FLG ) ==0 ) +return 0 ; + +printk (KERN_ERR "%s(): Not able to issue CPM command\n" , + __FUNCTION__); +return -EIO ;
Do these need to be protected with a spin lock?
Cheers,
g.
Signature · 4 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. [email protected] (403) 399-0195
Quoted 10 lines · On Thu, 22 Nov 2007 11:36:29 -0700 Grant Likely wrote:
Quoted 10 lines
arch/powerpc/sysdev/commproc.c +8
-0+int cpm_command (u32 command,u8 opcode) +{ +int i; + +if (command &0xffffff0f ) +return -EINVAL ; + + out_be16(&cpmp->cp_cpcr, command | CPM_CR_FLG | (opcode <<8));
+ for (i = 0; i <MAX_CR_CMD_LOOPS; i++)
Quoted 8 lines
arch/powerpc/sysdev/commproc.c void __init cpm_reset(void) · line 259
Quoted 6 lines
arch/powerpc/sysdev/commproc.c +6
-0+if ((in_be16 (&cpmp->cp_cpcr) &CPM_CR_FLG ) ==0 ) +return 0 ; + +printk (KERN_ERR "%s(): Not able to issue CPM command\n" , + __FUNCTION__); +return -EIO ;Do these need to be protected with a spin lock?
Even that might be not enough - we may have simultaneous call of this func in non-smp case... I was thinking of some kind of refcount, so one that is going to issue CPM command, must do say pq_cpmp_get() and another driver won't be able to mangle with cpcr while it's not done with previous request.
Yet I am not telling it was better the way it used to be - this approach looks okay but needs some efforts to defend against deadlocks while we are at it.
Signature · 1 line
Sincerely, Vitaly
Hi Vitaly,
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
arch/powerpc/sysdev/commproc.c +3
-0+printk (KERN_ERR "%s(): Not able to issue CPM command\n" , + __FUNCTION__); +return -EIO ;
Quoted 8 lines
Quoted 2 lines
Do these need to be protected with a spin lock?
Even that might be not enough - we may have simultaneous call of this func in non-smp case... I was thinking of some kind of refcount, so one that is going to issue CPM command, must do say
pq_cpmp_get()and another driver won't be able to mangle with cpcr while it's not done with previous request.Yet I am not telling it was better the way it used to be - this approach looks okay but needs some efforts to defend against deadlocks while we are at it
Wouldn't spin_lock_irqsave() prevent a deadlock?
Thanks,
Jochen
Quoted 14 lines · On Sat, 24 Nov 2007 18:53:34 +0100 Jochen Friedrich wrote:
Hi Vitaly,
Quoted 12 lines
Quoted 6 lines
arch/powerpc/sysdev/commproc.c void __init cpm_reset(void) · line 259
Quoted 4 lines
arch/powerpc/sysdev/commproc.c +3
-0+printk (KERN_ERR "%s(): Not able to issue CPM command\n" , + __FUNCTION__); +return -EIO ;Do these need to be protected with a spin lock?
Even that might be not enough - we may have simultaneous call of this func in non-smp case... I was thinking of some kind of refcount, so one that is going to issue CPM command, must do say
pq_cpmp_get()and another driver won't be able to mangle with cpcr while it's not done with previous request.
Quoted 6 lines
Quoted 3 lines
Yet I am not telling it was better the way it used to be - this approach looks okay but needs some efforts to defend against deadlocks while we are at it
Wouldn't
spin_lock_irqsave()prevent a deadlock?
yes, I believe it is OK for now.
Quoted 2 lines
Thanks,
Jochen
Signature · 1 line
Sincerely, Vitaly
Quoted 2 lines · Vitaly Bordug
Even that might be not enough - we may have simultaneous call of this func in non-smp case...
Do you really think that every piece of code that uses spinlocks in the kernel is broken on non-SMP?
Quoted 3 lines
I was thinking of some kind of refcount, so one that is going to issue CPM command, must do say
pq_cpmp_get()and another driver won't be able to mangle with cpcr while it's not done with previous request.
How on earth are you going to effect mutual exclusion using reference counting?
-Scott
Quoted 7 lines · On Mon, 26 Nov 2007 10:24:46 -0600 Scott Wood wrote:
Quoted 2 lines · Vitaly Bordug
Even that might be not enough - we may have simultaneous call of this func in non-smp case...
Do you really think that every piece of code that uses spinlocks in the kernel is broken on non-SMP?
No. I think spinlock is not universal save thing in such cases. See below.
Quoted 8 lines
Quoted 4 lines
I was thinking of some kind of refcount, so one that is going to issue CPM command, must do say
pq_cpmp_get()and another driver won't be able to mangle with cpcr while it's not done with previous request.How on earth are you going to effect mutual exclusion using reference counting?
perhaps I was not clear enough. That was a rough idea how to handle the whole thing, not just cpm_cr_cmd. This cpm command is a corner case, but there can be other actions that may confuse CPM being triggered simultaneously or overlapping. This is part of much bigger problem, and I was intended to have a look what people think about that.
Signature · 1 line
Sincerely, Vitaly
Quoted 4 lines · Vitaly Bordug
perhaps I was not clear enough. That was a rough idea how to handle the whole thing, not just
cpm_cr_cmd. This cpm command is a corner case, but there can be other actions that may confuse CPM being triggered simultaneously or overlapping.
What kind of actions did you have in mind? Microcode patching?
-Scott
Quoted 8 lines · On Mon, 26 Nov 2007 15:41:19 -0600 Scott Wood wrote:
Quoted 4 lines · Vitaly Bordug
perhaps I was not clear enough. That was a rough idea how to handle the whole thing, not just
cpm_cr_cmd. This cpm command is a corner case, but there can be other actions that may confuse CPM being triggered simultaneously or overlapping.What kind of actions did you have in mind? Microcode patching?
microcode is another case to handle gracefully. There are also
"soft" cases like cpmux mess-up, incompatible SoC devices (which we are handling by logical exclude now but which do have natural reasons of "not leaving together" like shared dedicated GPIO or smth else) etc.
Signature · 1 line
Sincerely, Vitaly