[U-Boot] [PATCH 3/3] board/seco: Add mx6q-uq7 basic board support

[Boris Brezillon <boris.brezillon@free-electrons.com>]

Hi Stefano,

On Mon, 02 Mar 2015 11:17:42 +0100
Stefano Babic <sbabic@denx.de> wrote:

> Hi Boris,
> 
> On 16/02/2015 14:27, Boris Brezillon wrote:
> > Add basic SECO MX6Q/uQ7 board support (Ethernet, UART, SD are supported).
> > It also adds a Kconfig skeleton to later add more SECO board (supporting
> > SoC and board variants).
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> >  arch/arm/cpu/armv7/mx6/Kconfig    |  11 +++
> >  board/seco/Kconfig                |  63 ++++++++++++++
> >  board/seco/common/Makefile        |   2 +
> >  board/seco/common/mx6.c           | 137 ++++++++++++++++++++++++++++++
> >  board/seco/common/mx6.h           |   9 ++
> >  board/seco/mx6quq7/Makefile       |   7 ++
> >  board/seco/mx6quq7/mx6quq7-2g.cfg | 173 ++++++++++++++++++++++++++++++++++++++
> >  board/seco/mx6quq7/mx6quq7.c      | 165 ++++++++++++++++++++++++++++++++++++
> >  configs/secomx6quq7_defconfig     |   7 ++
> >  include/configs/secomx6quq7.h     | 162 +++++++++++++++++++++++++++++++++++
> 
> MAINTAINERS file is missing.
> 
> I have tried your patches, I think there is something not coherent with
> names and the board cannot be built clean with buildman. Board is
> recognized as secomx6quq7, but then there is a mismatch in the board
> object library.
> 
> You set:
> 
> obj-y  := mx6q-uq7.o
> 
> 
> but the source is mx6quq7.c and building breaks with
> 
> make[1]: *** No rule to make target `board/seco/mx6quq7/mx6q-uq7.o',
> needed by `board/seco/mx6quq7/built-in.o'.  Stop.

Indeed.
I renamed all the files to match what's done for other boards (no
dashes or underscores in names), but apparently forgot to change it in
the Makefile.
I didn't notice it when compiling because I still add an mx6q-uq7.o file
in my directory.

Anyway, I'll fix that.

[...]

> > +void seco_mx6_setup_usdhc_iomux(int id)
> > +{
> > +	switch (id) {
> > +	case 3:
> > +		imx_iomux_v3_setup_multiple_pads(usdhc3_pads,
> > +						 ARRAY_SIZE(usdhc3_pads));
> > +		break;
> > +
> > +	case 4:
> > +		imx_iomux_v3_setup_multiple_pads(usdhc4_pads,
> > +						 ARRAY_SIZE(usdhc4_pads));
> > +		break;
> > +
> > +	default:
> 
> Maybe it is better to print an error due to wrong id.

Sure, I'll add an error message.


[...]

> > +int board_mmc_init(bd_t *bis)
> > +{
> > +	int status = 0;
> > +	u32 index = 0;
> > +
> > +	/*
> > +	 * Following map is done:
> > +	 * (U-boot device node)    (Physical Port)
> > +	 * mmc0                    eMMC on Board
> > +	 * mmc1                    Ext SD
> > +	 */
> > +	for (index = 0; index < CONFIG_SYS_FSL_USDHC_NUM; ++index) {
> > +		switch (index) {
> > +		case 0:
> > +			seco_mx6_setup_usdhc_iomux(3);
> > +			usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
> > +			usdhc_cfg[0].max_bus_width = 4;
> > +			break;
> > +		case 1:
> > +			seco_mx6_setup_usdhc_iomux(4);
> > +			usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK);
> > +			usdhc_cfg[1].max_bus_width = 4;
> > +			break;
> > +
> > +		default:
> > +			printf("Warning: %d exceed maximum number of SD ports %d\n",
> > +			       index + 1, CONFIG_SYS_FSL_USDHC_NUM);
> > +			return status;
> > +		}
> > +
> > +		status |= fsl_esdhc_initialize(bis, &usdhc_cfg[index]);
> 
> Fabio cleaned up all for all boards these statement, check for example
> the sabresd. Instead of cumulating the error, the loop stops at the
> first failure. Do the same to be coherent.

Okay, I'll have a look.

> 
> > +	}
> > +
> > +	return status;
> > +}
> > +
> > +int board_init(void)
> > +{
> > +	/* address of boot parameters */
> > +	gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
> > +
> > +	imx_iomux_v3_setup_pad(MX6_PAD_NANDF_D4__GPIO2_IO04 |
> > +			       MUX_PAD_CTRL(NO_PAD_CTRL));
> > +
> > +	gpio_direction_output(IMX_GPIO_NR(2, 4), 0);
> > +
> 
> Is it reset for phy ? Can you add a comment to explain what it is ?

I don't know what this pin is controlling, but if I don't do the
following toggle sequence, the board hangs.

Note that I don't have the schematics of this board, and all my work is
based on SECO's BSP.

> 
> > +	/* Set Low */
> > +	gpio_set_value(IMX_GPIO_NR(2, 4), 0);
> > +	udelay(1000);
> > +
> > +	/* Set High */
> > +	gpio_set_value(IMX_GPIO_NR(2, 4), 1);
> > +
> > +	return 0;
> > +}
> > +
> > +int board_late_init(void)
> > +{
> > +	return 0;
> > +}
> 
> Drop it, as well as CONFIG_BOARD_LATE_INIT

Ok.

[...]

> > +#define CONFIG_EXTRA_ENV_SETTINGS					\
> > +	"netdev=eth0\0"						\
> > +	"ethprime=FEC0\0"					\
> > +	"netdev=eth0\0"						\
> > +	"ethprime=FEC0\0"					\
> > +	"uboot=u-boot.bin\0"					\
> > +	"kernel=uImage\0"					\
> > +	"nfsroot=/opt/eldk/arm\0"				\
> > +	"ip_local=10.0.0.5::10.0.0.1:255.255.255.0::eth0:off\0"	\
> > +	"ip_server=10.0.0.1\0"					\
> > +	"nfs_path=/targetfs \0"					\
> > +	"memory=mem=1024M\0"					\
> > +	"bootdev=mmc dev 0; ext2load mmc 0:1\0"			\
> > +	"root=root=/dev/mmcblk0p1\0"				\
> > +	"option=rootwait rw fixrtc rootflags=barrier=1\0"	\
> > +	"cpu_freq=arm_freq=996\0"				\
> > +	"setbootargs=setenv bootargs console=ttymxc1,115200 ${root} ${option} ${memory} ${cpu_freq}\0"	\
> > +	"setbootargs_nfs=setenv bootargs console=ttymxc1,115200 root=/dev/nfs  nfsroot=${ip_server}:${nfs_path} nolock,wsize=4096,rsize=4096  ip=:::::eth0:dhcp  ${memory} ${cpu_freq}\0"	\
> 
> checkpatch is not very happy with these very long lines, can you fix
> them, please ?

Yes I know, I just wasn't sure if there was a rule like 'do not split
bootargs definitions in multi-line strings'.
I'll split the lines appropriately.

> 
> > +	"setbootdev=setenv boot_dev ${bootdev} 10800000 /boot/uImage\0"	\
> > +	"bootcmd=run setbootargs; run setbootdev; run boot_dev;  bootm 0x10800000\0"	\
> > +	"stdin=serial\0"					\
> > +	"stdout=serial\0"					\
> > +	"stderr=serial\0"
> > +
> > +
> > +/* Miscellaneous configurable options */
> > +#define CONFIG_SYS_LONGHELP
> > +#define CONFIG_SYS_HUSH_PARSER
> > +#define CONFIG_SYS_PROMPT		"SECO MX6Q uQ7 U-Boot > "
> > +
> > +#define CONFIG_AUTO_COMPLETE
> > +#define CONFIG_SYS_CBSIZE		256
> > +
> > +/* Print Buffer Size */
> > +#define CONFIG_SYS_PBSIZE		(CONFIG_SYS_CBSIZE +		\
> > +					 sizeof(CONFIG_SYS_PROMPT) + 16)
> > +#define CONFIG_SYS_MAXARGS		16
> > +#define CONFIG_SYS_BARGSIZE		CONFIG_SYS_CBSIZE
> > +
> > +#define CONFIG_SYS_LOAD_ADDR		CONFIG_LOADADDR
> > +#define CONFIG_SYS_HZ			1000
> > +
> > +#define CONFIG_CMDLINE_EDITING
> > +
> > +
> > +/* Physical Memory Map */
> > +#define CONFIG_NR_DRAM_BANKS		1
> > +#define PHYS_SDRAM			MMDC0_ARB_BASE_ADDR
> > +#define PHYS_SDRAM_SIZE			(2u * 1024 * 1024 * 1024)
> > +
> > +#define CONFIG_SYS_SDRAM_BASE		PHYS_SDRAM
> > +#define CONFIG_SYS_INIT_RAM_ADDR	IRAM_BASE_ADDR
> > +#define CONFIG_SYS_INIT_RAM_SIZE	IRAM_SIZE
> > +
> > +#define CONFIG_SYS_INIT_SP_OFFSET	\
> > +	(CONFIG_SYS_INIT_RAM_SIZE - GENERATED_GBL_DATA_SIZE)
> > +#define CONFIG_SYS_INIT_SP_ADDR		\
> > +	(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET)
> > +
> > +/* FLASH and environment organization */
> > +#define CONFIG_SYS_NO_FLASH
> > +
> > +#define CONFIG_ENV_SIZE			(8 * 1024)
> > +
> > +#if defined(CONFIG_ENV_IS_IN_MMC)
> > +	#define CONFIG_ENV_OFFSET		(6 * 128 * 1024)
> > +	#define CONFIG_SYS_MMC_ENV_DEV		0
> > +	#define CONFIG_DYNAMIC_MMC_DEVNO
> > +#endif
> > +
> > +#define CONFIG_OF_LIBFDT
> > +#define CONFIG_CMD_BOOTZ
> > +
> > +#ifndef CONFIG_SYS_DCACHE_OFF
> > +#define CONFIG_CMD_CACHE
> > +#endif
> > +
> > +#endif /* __CONFIG_H */
> > 
> 

Thanks for your review.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com