Re: [RFC/PATCHv3] powerpc: Move CPM command handling into the cpm drivers

[Scott Wood <scottwood@freescale.com>]

Jochen Friedrich wrote:
> diff --git a/arch/powerpc/sysdev/commproc.c 
> b/arch/powerpc/sysdev/commproc.c
> index f6a6378..2426bcb 100644
> --- a/arch/powerpc/sysdev/commproc.c
> +++ b/arch/powerpc/sysdev/commproc.c
> @@ -240,6 +240,34 @@ void __init cpm_reset(void)
> #endif
> }
> 
> +static DEFINE_SPINLOCK(cmd_lock);
> +
> +#define MAX_CR_CMD_LOOPS        10000
> +
> +int cpm_command(u32 command, u8 opcode)
> +{
> +    int i, ret;
> +    unsigned long flags;
> +
> +    if (command & 0xffffff0f)
> +        return -EINVAL;
> +
> +    spin_lock_irqsave(&cmd_lock, flags);

I'm a little uneasy about this -- what's the longest it can take for the 
CPM command to complete (or timeout)?  I'd rather use a mutex, if we can 
make sure that it's never called from atomic context.

> +
> +    ret = 0;
> +    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)
> +            goto out;
> +
> +    printk(KERN_ERR "%s(): Not able to issue CPM command\n", 
> __FUNCTION__);
> +    ret = -EIO;
> +out:
> +    spin_unlock_irqrestore(&cmd_lock, flags);
> +    return ret;
> +}
> +EXPORT_SYMBOL_GPL(cpm_command);

Can we please not play the GPL DRM game here?

Besides, Documentation/DocBook/kernel-hacking.tmpl explicitly says that 
"It implies that the function is considered an internal implementation 
issue, and not really an interface."

I don't think this is the case here.

-Scott