On Fri, Oct 01, 2010 at 07:16:53AM -0500, Peter Ujfalusi wrote:
On Friday 01 October 2010 14:50:37 David Cohen wrote:

> +          l = sys_cf = dma_read(OCP_SYSCONFIG);

Shouldn't it be avoided?

I'm restoring the OCP_SYSCONFIG register later on, and reusing 'l' here to avoid
confusing expression when moving the sDMA to NoStandby.
I could as well introduce other variable for this...


Regards,

David

> +          l &= ~DMA_SYSCONFIG_MIDLEMODE_MASK;
> +          l |= DMA_SYSCONFIG_MIDLEMODE(DMA_IDLEMODE_NO_IDLE);
> +          dma_write(l , OCP_SYSCONFIG);

I could have:
        dma_write((sys_fc & (~DMA_SYSCONFIG_MIDLEMODE_MASK)) |
                         DMA_SYSCONFIG_MIDLEMODE(DMA_IDLEMODE_NO_IDLE), 
OCP_SYSCONFIG);

Than I do not need the re-read the CCR, but it looks a bit messy.

I think David is concerned about multiple assigments in one line.
CodingStyle asks to avoid that. So probably:

sys_cf = dma_read(OCP_SYSCONFIG);
l = sys_cf;

would make chekcpatch.pl --strict happy ??

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to