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

[Stefano Babic]

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.

> +
> +#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 ?

> 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.


> 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.


> +/* 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 ?

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================