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?
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.
Please also keep in mind that CONFIG_* options should be documented in the README.
Thanks.
Best regards,
Wolfgang Denk