Re: [PATCH v2 4/8] arm: kirkwood: Pogoplug-V4 : Add board include configs file

[Stefan Roese <sr@denx.de>]

On 12/23/21 08:26, Tony Dinh wrote:
> Add include configs file for Pogoplug V4 board
> 
> Signed-off-by: Tony Dinh <mibodhi@gmail.com>
> ---
> 
> (no changes since v1)
> 
>   include/configs/pogo_v4.h | 94 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 94 insertions(+)
>   create mode 100644 include/configs/pogo_v4.h
> 
> diff --git a/include/configs/pogo_v4.h b/include/configs/pogo_v4.h
> new file mode 100644
> index 0000000000..b449986e8c
> --- /dev/null
> +++ b/include/configs/pogo_v4.h
> @@ -0,0 +1,94 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2014-2021 Tony Dinh <mibodhi@gmail.com>
> + *
> + * Based on
> + * Copyright (C) 2012
> + * David Purdy <david.c.purdy@gmail.com>
> + *
> + * Based on Kirkwood support:
> + * (C) Copyright 2009
> + * Marvell Semiconductor <www.marvell.com>
> + * Written-by: Prafulla Wadaskar <prafulla@marvell.com>
> + */
> +
> +#ifndef _CONFIG_POGO_V4_H
> +#define _CONFIG_POGO_V4_H
> +
> +/*
> + * Machine type definition and ID
> + */
> +#define MACH_TYPE_POGO_V4		3960
> +#define CONFIG_MACH_TYPE		MACH_TYPE_POGO_V4

Are you really using the pretty old CONFIG_MACH_TYPE to boot Linux
on this device?

> +
> +/*
> + * High Level Configuration Options (easy to change)
> + */
> +#define CONFIG_FEROCEON_88FR131		/* #define CPU Core subversion */
> +#define CONFIG_KW88F6192		/* SOC Name */

At least CONFIG_FEROCEON_88FR131 is available in Kconfig
(arch/arm/mach-kirkwood/Kconfig), so there should be no need to add it
via some header.

Please double-check other macros in this header as well for availability
in Kconfig.

> +
> +/*
> + * Commands configuration
> + */
> +#define CONFIG_SYS_NO_FLASH		/* Declare no flash (NOR/SPI) */
> +#define CONFIG_SYS_PROMPT_HUSH_PS2 "> "
> +
> +/*
> + * mv-common.h should be defined after CMD configs since it used them
> + * to enable certain macros
> + */
> +#include "mv-common.h"
> +
> +/*
> + * Default environment variables
> + */
> +#define CONFIG_BOOTCOMMAND \
> +	"setenv bootargs $(bootargs_console); " \
> +	"run bootcmd_usb; " \
> +	"bootm 0x00800000 0x01100000 0x2c00000"
> +
> +#define CONFIG_EXTRA_ENV_SETTINGS \
> +	"dtb_file=/boot/dts/" CONFIG_DEFAULT_DEVICE_TREE ".dtb\0" \
> +	"mtdparts=" CONFIG_MTDPARTS_DEFAULT "\0"\
> +	"mtdids=nand0=orion_nand\0"\
> +	"bootargs_console=console=ttyS0,115200\0" \
> +	"bootcmd_usb=usb start; load usb 0:1 0x00800000 /boot/uImage; " \
> +	"load usb 0:1 0x01100000 /boot/uInitrd; " \
> +	"load usb 0:1 0x2c00000 $dtb_file\0"
> +
> +/*
> + * Ethernet Driver configuration
> + */
> +#ifdef CONFIG_CMD_NET
> +#define CONFIG_FEATURE_COMMAND_EDITING	/* for netconsole */
> +#define CONFIG_MVGBE_PORTS	{1, 0}	/* enable port 0 only */
> +#define CONFIG_PHY_BASE_ADR	0
> +#endif /* CONFIG_CMD_NET */
> +
> +/*
> + * File system
> + */
> +#ifdef CONFIG_CMD_JFFS2
> +#define CONFIG_JFFS2_NAND
> +#define CONFIG_JFFS2_LZO
> +#endif /* CONFIG_CMD_JFFS2 */
> +
> +/*
> + *  SATA Driver configuration
> + */
> +#ifdef CONFIG_SATA
> +#define CONFIG_SYS_SATA_MAX_DEVICE	1

This is also in Kconfig. Please make sure to not add any ad-hoc macros
here that are Kconfig options now.

Also, please drop the "#ifdef CONFIG_SATA" (and other similar ones) if
it's not really select-able.

Thanks,
Stefan

> +#endif /* CONFIG_SATA */
> +
> +/*
> + * Support large disk for SATA and USB
> + */
> +#define CONFIG_SYS_64BIT_LBA
> +#define CONFIG_LBA48
> +
> +/*
> + * Kirkwood GPIO
> + */
> +#define CONFIG_KIRKWOOD_GPIO
> +
> +#endif /* _CONFIG_POGO_V4_H */
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de