[U-Boot] [PATCH v2 1/6] arm: mvf600: Add Vybrid MVF600 CPU support

[Wang Huan-B18965]

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 <b18965 at freescale.com>
> > ---
> 
> 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