[PATCH 1/2] video: Add i.MX23/28 framebuffer driver
jbe at pengutronix.de
Thu Feb 10 06:12:17 EST 2011
Li Frank-B20596 wrote:
> > 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.
Its your opinion...
> > > 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.
If you collect everything in header files: Yes. If you keep the macros where
they are used, IMHO: No.
> VDCTRL4_SET_DOTCLK_DLY(x), according to our macro policy, it should be
> Generally, you need
> Reg = readl(offset+VDCTRL4);
> Reg &=~ ((0x7)<<29);
> Reg &= ~VDCTRL4_SET_DOTCLK_DLY(0x7);
> Reg &= ~VDCTRL4_SET_DOTCLK_DLY_MASK;
Okay, the even get longer and longer.
> 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)
Anywhere else such naming policy in the kernel? I have to deal with all kind
of CPUs, not only i.MX23/28.
Pengutronix e.K. | Juergen Beisert |
Linux Solutions for Science and Industry | Phone: +49-8766-939 228 |
Vertretung Sued/Muenchen, Germany | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de/ |
More information about the linux-arm-kernel