
On 11/14/2011 10:42 AM, Jason Hui wrote:
All get_usdhcX function are identical, except for two masks (MXC_CCM_CSCDR1_USDHCx_PODF_MASK and MXC_CCM_CSCMR1_USDHCx_CLK_SEL). Merge them in a get_usdhc_clk(usdhc_number)
Yes, I also notice this when writing the code. At that time, I just think to make clear about the code. Now, I think your comments is also reasonable. I will change it later.
ok
+#if defined(CONFIG_FEC_MXC) +void imx_get_mac_from_fuse(unsigned char *mac) +{
struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;
struct fuse_bank *bank = &iim->bank[4];
struct fuse_bank4_regs *fuse =
(struct fuse_bank4_regs *)bank->fuse_regs;
u32 mac_lo = readl(&fuse->mac_addr_low);
u32 mac_hi = readl(&fuse->mac_addr_high);
*(u32 *)mac = mac_lo;
mac[4] = mac_hi & 0xff;
mac[5] = (mac_hi >> 8) & 0xff;
+}
Even if the implementation is slightly different, this function (except for the bank number) does the same things for MX5 and MX6 - you can als put it into imx-common.
Stefano, there is big difference for the memory layout. On i.mx5, the layout is:
32-bit 32-bit 32-bit 32-bit 32-bit 32-bit mac0 mac1 mac2 mac3 mac4 mac5
but on i.mx6:
32bit 32bit [low 16bit] mac[0-3] mac[4-5]
It's due to i.mx6q use OCOTP fuse-controller other than the fuse-box on the i.mx5.
Thus, I think, I will keep the code as it is.
Ok - I was not aware about it.
Checking this there something confusing in actual code. There is struct mxc_ccm_reg in crm_regs.h and struct clkctl in imx-regs.h. The two
I did not searched the clkctl in the patch-set by using: grep -nR clkctl *
structure are identical. I am asking myself why...it seems to me that someting went wrong. If there is no evident reason, we should even clean up this point.
And it is better you use one of the already supplied names (mxc_ccm_reg or clkctl), without adding a new one.
Yes, I'm using mxc_ccm_reg.
Ok - this structure is also used in most drivers - if I am not wrong (and I will better check), the struct clkctl is used only to generate the offsets in asm-offsets.h. If it is so, it could be drop in a future clean up patch...
Maybe do we find a way to add a common include directory ? This file is duplicated. We can use include/asm/arch/imx-common
I don't find one good way to add a common include directory. If I try to find one, I will put this head file to imx-common.
The easy way is to add include/asm/arch-imx-common, and then the header are explicitely included as #include <asm/arch-imx-common/..>. I have seen only another example in U-Boot for armv7:
arch/arm/cpu/armv7/highbank/timer.c:#include <asm/arch-armv7/systimer.h>
I am opened to other solutions, too, but I think that introducing a common repository for IMX include files (as Linux with plat_imx does) is the way to do.
Best regards, Stefano Babic