Hi, Wolfgang,
-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Saturday, October 16, 2010 3:08 AM To: Liu Hui-R64343 Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH V2 1/1] IMX: rename mx51 to mx5
Dear Jason Liu,
In message 1287155158-16801-1-git-send-email-r64343@freescale.com
you
wrote: ...
- freq = decode_pll(mxc_plls[PLL1_CLOCK], CONFIG_MX51_HCLK_FREQ);
- freq = decode_pll(mxc_plls[PLL1_CLOCK], CONFIG_HCLK_FREQ);
...
- clockinfo, CONFIG_SYS_MAXARGS, 1,
do_mx51_showclocks,
- "display mx51 clocks\n",
- clockinfo, CONFIG_SYS_MAXARGS, 1, do_showclocks,
- "display clocks\n",
...
- lastinc = val / (CONFIG_MX51_CLK32 / CONFIG_SYS_HZ);
- lastinc = val / (CONFIG_CLK32 / CONFIG_SYS_HZ);
I'm not happy about these changes.
The old names, containing "MX51" may have been too narrow in their naming; but the new names are now way too general.
"CONFIG_CLK32" or "CONFIG_HCLK_FREQ" look like generic config options that apply to any processor or board, or at least potentially to a wider range. This is kind of name space pollution.
I understand that all these names are specific to the mx5 family of processors only, right?
Yes, specific to mx5 family,
Recent submissions to the Linux kernel seem to use a "MX5" string when referring to these processors, like in:
commit messages:
ARM: mx5: Add Nand clock support ARM: mx5: dynamically register mxc-nand device ARM: mx5: dynamically register imx-i2c devices
directory names:
arch/arm/mach-mx5/
preprocessor variables:
MX5_USBOTHER_REGS_OFFSET MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK
etc.
I suggest we do the same. Please add a "mx5" resp. "MX5" to the new identifier names.
Yes, agree.
Please also keep in mind that CONFIG_* options should be documented in the README.
Do you mean all the specific CONFIG_* options in include/configs/boardname.h should be documented? I found lots of that is not doing what you called for? :(
BTW, which definition is better?
CONFIG_SYS_MX5_HCLK_FREQ 24000000 CONFIG_MX5_HCLK_FREQ 24000000
According to the REAME, I prefer to the former, what do you think.
Thanks.
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de "May your future be limited only by your dreams."
- Christa McAuliffe