[PATCH 1/2] video: Add i.MX23/28 framebuffer driver

Li Frank-B20596 B20596 at freescale.com
Thu Feb 10 05:37:51 EST 2011


> This kind of macro encryption _may_ help when you are coding the driver
> the
> first time. But after reading and reading it again (while testing and
> debugging) all these prefixes and suffixes do not add any valuable
> information. They only fill up your lines, enlarges your source code and
> make
> you blind for the real bug...
> 
> Everyone who wants to see how source code looks that uses these longs
> macros I
> can recommend reading the so called 'bootlets' source code. :-))

The complex of bootlets is not long macro, but the work around to make sure
Chip PMU work safely. If there are not such macro, power_prep will become
more difficult to understand.

> 
> > Developer needn't look up register header file when coding, just
> 
> That's why I add these macros into the source file instead into a header
> file.
> 
> > write down Register name or bit name according to mx23/mx28 spec.
> 
> And sometimes the spec is incomplete or just wrong and so on. Independent
> of
> any macro naming policy.

Not at all. Mx23/mx28 can keep spec, header file and RTL consistent just
because using one source and multiple output. During Freescale mx28 BSP
develop, never happen bit and register definition error.  
 
> 
> > If you still think it is too long, I suggest keep HW_ and BM_ prefix to
> > distinguish Which one is register name, which one is bit mask.
> 
> The used macro in the address part of the writel() must be an offset,
> while
> the macro used in the value part must be a bit definition. Anything else
> is
> redundant.

If name is too short, there are possible to cause name pollution. 
BM_, BP_, BF_ have their implication.

VDCTRL4_SET_DOTCLK_DLY(x), according to our macro policy, it should be
BF_VDCTRL4_SET_DOTCLK_DLY(x). 

Generally, you need 
Reg = readl(offset+VDCTRL4);
Reg &=~ ((0x7)<<29);
Reg |= VDCTRL4_SET_DOTCLK_DLY(value)
Writel(reg, offset+VDCTRL4)

Compared with our macro policy.

Reg = readl(offset+HW_VDCTRL4);
Reg &=~ BM_VDCTRL4_SET_DOTCLK_DLY;
Reg |= BF_VDCTRL4_SET_DOTCLK_DLY(value)
Writel(reg, offset+HW_VDCTRL4)


> 
> Regards,
> Juergen
> 




More information about the linux-arm-kernel mailing list