[2/5] powerpc/qe: Make qe_reset() code path safe for repeated invocation

For MPC8569 CPUs we'll need to reset QE after each suspend, so make
qe_reset() code path suitable for repeated invocation, that is:

- Don't initialize rheap structures if already initialized;
- Don't allocate muram for SDMA if already allocated, just reinitialize
  registers with previously allocated muram offset;

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/sysdev/cpm_common.c |    3 +++
 arch/powerpc/sysdev/qe_lib/qe.c  |   10 ++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

On Aug 27, 2009, at 12:30 PM, Anton Vorontsov wrote:

>
> For MPC8569 CPUs we'll need to reset QE after each suspend, so make
> qe_reset() code path suitable for repeated invocation, that is:
>
> - Don't initialize rheap structures if already initialized;
> - Don't allocate muram for SDMA if already allocated, just  
> reinitialize
>  registers with previously allocated muram offset;
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/sysdev/cpm_common.c |    3 +++
> arch/powerpc/sysdev/qe_lib/qe.c  |   10 ++++++----
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/ 
> cpm_common.c
> index e4b6d66..ddfca52 100644
> --- a/arch/powerpc/sysdev/cpm_common.c
> +++ b/arch/powerpc/sysdev/cpm_common.c
> @@ -81,6 +81,9 @@ int __init cpm_muram_init(void)
> 	int i = 0;
> 	int ret = 0;
>
> +	if (muram_pbase)
> +		return 0;
> +
> 	spin_lock_init(&cpm_muram_lock);
> 	/* initialize the info header */
> 	rh_init(&cpm_muram_info, 1,
> diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/ 
> qe_lib/qe.c
> index b06564f..f485d5a 100644
> --- a/arch/powerpc/sysdev/qe_lib/qe.c
> +++ b/arch/powerpc/sysdev/qe_lib/qe.c
> @@ -317,16 +317,18 @@ EXPORT_SYMBOL(qe_put_snum);
> static int qe_sdma_init(void)
> {
> 	struct sdma __iomem *sdma = &qe_immr->sdma;
> -	unsigned long sdma_buf_offset;
> +	static unsigned long sdma_buf_offset;
>
> 	if (!sdma)
> 		return -ENODEV;
>
> 	/* allocate 2 internal temporary buffers (512 bytes size each) for
> 	 * the SDMA */
> - 	sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
> -	if (IS_ERR_VALUE(sdma_buf_offset))
> -		return -ENOMEM;
> +	if (!sdma_buf_offset) {
> +		sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
> +		if (IS_ERR_VALUE(sdma_buf_offset))

shouldn't we zero out sdma_buf_offset otherwise if we call this again  
we'll think its set.

> +			return -ENOMEM;
> +	}
>
> 	out_be32(&sdma->sdebcr, (u32) sdma_buf_offset & QE_SDEBCR_BA_MASK);
>  	out_be32(&sdma->sdmr, (QE_SDMR_GLB_1_MSK |
> -- 
> 1.6.3.3
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

On Fri, Aug 28, 2009 at 12:34:50AM -0500, Kumar Gala wrote:
[...]
> >static int qe_sdma_init(void)
> >{
> >	struct sdma __iomem *sdma = &qe_immr->sdma;
> >-	unsigned long sdma_buf_offset;
> >+	static unsigned long sdma_buf_offset;
> >
> >	if (!sdma)
> >		return -ENODEV;
> >
> >	/* allocate 2 internal temporary buffers (512 bytes size each) for
> >	 * the SDMA */
> >- 	sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
> >-	if (IS_ERR_VALUE(sdma_buf_offset))
> >-		return -ENOMEM;
> >+	if (!sdma_buf_offset) {
> >+		sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
> >+		if (IS_ERR_VALUE(sdma_buf_offset))
> 
> shouldn't we zero out sdma_buf_offset otherwise if we call this
> again we'll think its set.

Technically, no. If qe_sdma_init() fails, kernel will panic:

void __init qe_reset(void)
{
        ...
        if (qe_sdma_init())
                panic("sdma init failed!");
}

But I see your point, it isn't obvious and may lead to a bug if
we'll decide to not panic later on. Therefore I'd better make the
change.

Thanks,