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

[Anton Vorontsov <avorontsov@ru.mvista.com>]

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,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2