Re: [PATCH 01/11] OMAP: DMA: Introduce DMA device attributes

["Cousson, Benoit" <b-cousson@ti.com>]

On 7/30/2010 5:57 AM, G, Manjunath Kondaiah wrote:

<snip>

>>> +/* Attributes for OMAP DMA Contrllers */
>>> +#define ENABLE_1510_MODE		(1<<   0)
>>> +#define DMA_LINKED_LCH			(1<<   1)
>>> +#define GLOBAL_PRIORITY			(1<<   2)
>>> +#define RESERVE_CHANNEL			(1<<   3)
>>> +#define SRC_PORT			(2<<   3)
>>> +#define DST_PORT			(2<<   4)
>>> +#define IS_CSSA_32			(2<<   5)
>>> +#define IS_CDSA_32			(2<<   6)
>>> +#define SRC_INDEX			(4<<   6)
>>> +#define DST_INDEX			(4<<   7)
>>> +#define IS_BURST_ONLY4			(4<<   8)
>>> +#define CLEAR_CSR_ON_READ		(4<<   9)
>>> +#define IS_WORD_16			(8<<   9)
>>> +#define IS_RW_PRIORIY			(8<<   0xA)
>>
>> It is a minor detail, but why are you shifting both side of the
>> operator? It is really confusing, cannot you just do 1<<  X?
>> With 0<  X<  13.
> I can change this if it is confusing to others.

So you are still convince that this is a good way to do it???
What is the rational behind that?
Why adding potential source of confusion when you can avoid that easily?

Regards,
Benoit