Hi, Stefano, > > On 14/05/2013 11:51, Alison Wang wrote: > > This patch adds generic codes to support Freescale's Vybrid MVF600 CPU. > > > > It aligns Vybrid MVF600 platform with i.MX platform. As there are some > > differences between MVF600 and i.MX platforms, the specific codes are > > in the arch/arm/cpu/armv7/mvf600 directory. > > > > Signed-off-by: Alison Wang <[email protected]> > > --- > > Hi Alison, > > > Changes in v2: > > - Remove vybrid-common directory > > - Rename directory name 'vybrid' to 'mvf600' > > - Add generic.c file > > - Rewrite get_reset_cause() to make it readable > > - Remove reset_cpu(), and use the function in imx_watchdog.c > > - Rewrite timer.c file > > - Use vybrid_get_clock(VYBRID_UART_CLK) instead of > > vybrid_get_uartclk() > > - Remove lowlevel_init.S, and add clock_init() in board_early_init_f() > > - Remove useless CONFIG_SYS_ defines > > - Move CONFIG_MACH_TYPE to board configuration file > > - Define C structures and access C structures to set/read registers > > - Remove useless errata > > - Remove useless macros > > - Rename directory 'arch-vybrid' to 'arch-mvf600' > > > > Makefile | 2 +- > > arch/arm/cpu/armv7/mvf600/Makefile | 42 ++++ > > arch/arm/cpu/armv7/mvf600/generic.c | 309 > ++++++++++++++++++++++++++++ > > Just a minor concern here. The SOC is a ARMv5, but files go into the > armv7 directory. Maybe the bigger issue can be with the increasing number > of work-around (CONFIG_ERRATA) that flow into start.S for armv7, that are > specific only for armv7. I know that for ARMv5 we split differently > instead of ARM architecture (ARM926,...). > > Albert, what do you think about ? Should these files be moved away from > armv7 ? > > > +unsigned int mvf_get_clock(enum mvf_clock clk) { > > + switch (clk) { > > + case MVF_ARM_CLK: > > + return get_mcu_main_clk(); > > + case MVF_BUS_CLK: > > + return get_bus_clk(); > > + case MVF_IPG_CLK: > > + return get_ipg_clk(); > > + case MVF_UART_CLK: > > + return get_uart_clk(); > > + case MVF_ESDHC_CLK: > > + return get_sdhc_clk(); > > + case MVF_FEC_CLK: > > + return get_fec_clk(); > > + default: > > + break; > > + } > > + return -1; > > +} > > Ok - we have the same structure as for i.MX. I agree with you that the > name of the function mxc_get_clock() is not anymore correct, after some > other Freescale's SOC families were introduced. However, it is still > important to have a common API to expone a SOC to a board maintainer. > > If you see, the mxs family (MX23 / MX28) has a mxc_get_clock(), even if > most internal functions are marked as mxs_. I think we can later change > the name for this function (maybe this is not the only one) to make the > name clearer and not specific to i.MX, but then it is will be easier if > all SOCs use the same names. For this reason, it is better to rename this > function to mxc_get_clock() and please take the same enums that are > already set for the other Freescale's SOCs. [Alison Wang] Agree. I will rename this function to mxc_get_clock() and take the same enums. Thanks. > > > + > > +#ifdef CONFIG_FEC_MXC > > +void imx_get_mac_from_fuse(int dev_id, unsigned char *mac) { > > + struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR; > > + struct fuse_bank *bank = &ocotp->bank[4]; > > + struct fuse_bank4_regs *fuse = > > + (struct fuse_bank4_regs *)bank->fuse_regs; > > + > > + u32 value = readl(&fuse->mac_addr0); > > + mac[0] = (value >> 8); > > + mac[1] = value; > > To my knowledge : is the whole MAC stored in the ocotp ? No need to add > the first bytes (vendor-id) as we had for MX28 ? [Alison Wang] Yes, the whole MAC is stored in the ocotp for Vybrid. > > > diff --git a/arch/arm/cpu/armv7/mvf600/timer.c > > b/arch/arm/cpu/armv7/mvf600/timer.c > > new file mode 100644 > > index 0000000..99ca57d > > --- /dev/null > > +++ b/arch/arm/cpu/armv7/mvf600/timer.c > > @@ -0,0 +1,144 @@ > > +/* > > + * Copyright 2013 Freescale Semiconductor, Inc. > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License as > > + * published by the Free Software Foundation; either version 2 of > > + * the License, or (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program; if not, write to the Free Software > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, > > + * MA 02111-1307 USA > > + */ > > + > > +#include <common.h> > > +#include <asm/io.h> > > +#include <div64.h> > > +#include <asm/arch/imx-regs.h> > > +#include <asm/arch/clock.h> > > + > > +/* Periodic interrupt timer registers */ struct pit_reg { > > + u32 mcr; > > + u32 recv0[55]; > > + u32 ltmr64h; > > + u32 ltmr64l; > > + u32 recv1[6]; > > + u32 ldval0; > > + u32 cval0; > > + u32 tctrl0; > > + u32 tflg0; > > + u32 ldval1; > > + u32 cval1; > > + u32 tctrl1; > > + u32 tflg1; > > + u32 ldval2; > > + u32 cval2; > > + u32 tctrl2; > > + u32 tflg2; > > + u32 ldval3; > > + u32 cval3; > > + u32 tctrl3; > > + u32 tflg3; > > + u32 ldval4; > > + u32 cval4; > > + u32 tctrl4; > > + u32 tflg4; > > + u32 ldval5; > > + u32 cval5; > > + u32 tctrl5; > > + u32 tflg5; > > + u32 ldval6; > > + u32 cval6; > > + u32 tctrl6; > > + u32 tflg6; > > + u32 ldval7; > > + u32 cval7; > > + u32 tctrl7; > > + u32 tflg7; > > +}; > > + > > I had put these structure in imx-regs.h - no block from my side, but > there is also no big reason to let it here. [Alison Wang] Agree. I will put these structures to imx-regs.h. Thanks. > > > > diff --git a/arch/arm/include/asm/arch-mvf600/clock.h > > b/arch/arm/include/asm/arch-mvf600/clock.h > > new file mode 100644 > > index 0000000..889d4d9 > > --- /dev/null > > +++ b/arch/arm/include/asm/arch-mvf600/clock.h > > @@ -0,0 +1,38 @@ > > +/* > > + * Copyright 2013 Freescale Semiconductor, Inc. > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License as > > + * published by the Free Software Foundation; either version 2 of > > + * the License, or (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program; if not, write to the Free Software > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, > > + * MA 02111-1307 USA > > + */ > > + > > +#ifndef __ASM_ARCH_CLOCK_H > > +#define __ASM_ARCH_CLOCK_H > > + > > +#include <common.h> > > + > > +enum mvf_clock { > > + MVF_ARM_CLK = 0, > > + MVF_BUS_CLK, > > + MVF_IPG_CLK, > > + MVF_UART_CLK, > > + MVF_ESDHC_CLK, > > + MVF_FEC_CLK, > > +}; > > + > > +unsigned int mvf_get_clock(enum mvf_clock clk); > > + > > +#define imx_get_fecclk() mvf_get_clock(MVF_FEC_CLK) > > See my previous comment. Agree the names are not anymore correct, but we > can fix them later with a separate patch for all Freescale's SOCs. [Alison Wang] Agree. I will change the names. Thanks. > > > > +/* On-Chip One Time Programmable Controller (OCOTP) */ struct > > +ocotp_regs { > > + u32 ctrl; > > + u32 ctrl_set; > > + u32 ctrl_clr; > > + u32 ctrl_tog; > > + u32 timing; > > + u32 rsvd0[3]; > > + u32 data; > > + u32 rsvd1[3]; > > + u32 read_ctr; > > + u32 rsvd2[3]; > > + u32 read_fuse_data; > > + u32 rsvd3[7]; > > + u32 scs; > > + u32 scs_set; > > + u32 scs_clr; > > + u32 scs_tog; > > + u32 crc_addr; > > + u32 rsvd4[3]; > > + u32 crc_value; > > + u32 rsvd5[3]; > > + u32 version; > > + u32 rsvd6[0xdb]; > > + > > + struct fuse_bank { > > + u32 fuse_regs[0x20]; > > + } bank[16]; > > +}; > > + > > +/* OTP Bank 4 */ > > +struct fuse_bank4_regs { > > + u32 sjc_resp0; > > + u32 rsvd0[3]; > > + u32 sjc_resp1; > > + u32 rsvd1[3]; > > + u32 mac_addr0; > > + u32 rsvd2[3]; > > + u32 mac_addr1; > > + u32 rsvd3[3]; > > + u32 mac_addr2; > > + u32 rsvd4[3]; > > + u32 mac_addr3; > > + u32 rsvd5[3]; > > + u32 gp1; > > + u32 rsvd6[3]; > > + u32 gp2; > > + u32 rsvd7[3]; > > +}; > > + > > Have you seen that a driver for fuse / ocotp was recently added to > mainline ? Have you tested on your platform ? [Alison Wang] Yes, I saw the driver and tested it on my platform just now. It worked fine. I could enable ocotp support in the next version patch.
Thanks! Best Regards, Alison Wang _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
