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: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
