Subject: Re: powerpc/85xx: Add P1023RDB board support

From: Scott Wood <[email-masked]>
Date: Mon, 29 Jul 2013 20:09:45 -0500

On Fri, Jun 14, 2013 at 04:26:20PM +0800, Chunhe Lan wrote:
> P1023RDB Specification:
> -----------------------
> Memory subsystem:
>    512MB DDR3 (Fixed DDR on board)
>    64MB NOR flash
>    128MB NAND flash

Where is NAND in the device tree?

> Ethernet:
>    eTSEC1: Connected to Atheros AR8035 GETH PHY
>    eTSEC2: Connected to Atheros AR8035 GETH PHY

Where are the PHYs in the device tree?

> +			partition@3f00000 {
> +				label = "firmware";
> +				reg = <0x03f00000 0x00080000>;
> +				read-only;
> +			};
> +			partition@3f80000 {
> +				label = "u-boot";
> +				reg = <0x03f80000 0x00080000>;
> +				read-only;
> +			};

What is "firmware" if not U-Boot?  FMan firmware?  Note that part of your
"firmware" partition contains the U-Boot environment.

> diff --git a/arch/powerpc/configs/85xx/p1023rds_defconfig b/arch/powerpc/configs/85xx/p1023rds_defconfig
> index b80bcc6..092a746 100644
> --- a/arch/powerpc/configs/85xx/p1023rds_defconfig
> +++ b/arch/powerpc/configs/85xx/p1023rds_defconfig
> @@ -22,6 +22,7 @@ CONFIG_MODVERSIONS=y
>  # CONFIG_BLK_DEV_BSG is not set
>  CONFIG_PARTITION_ADVANCED=y
>  CONFIG_MAC_PARTITION=y
> +CONFIG_P1023_RDB=y
>  CONFIG_P1023_RDS=y
>  CONFIG_QUICC_ENGINE=y
>  CONFIG_QE_GPIO=y
> @@ -67,6 +68,7 @@ CONFIG_PROC_DEVICETREE=y
>  CONFIG_BLK_DEV_LOOP=y
>  CONFIG_BLK_DEV_RAM=y
>  CONFIG_BLK_DEV_RAM_SIZE=131072
> +CONFIG_EEPROM_AT24=y
>  CONFIG_EEPROM_LEGACY=y
>  CONFIG_BLK_DEV_SD=y
>  CONFIG_CHR_DEV_ST=y
> @@ -102,6 +104,7 @@ CONFIG_SERIAL_8250_RSA=y
>  CONFIG_SERIAL_QE=m
>  CONFIG_NVRAM=y
>  CONFIG_I2C=y
> +CONFIG_I2C_CHARDEV=y
>  CONFIG_I2C_CPM=m
>  CONFIG_I2C_MPC=y
>  CONFIG_GPIO_MPC8XXX=y
> @@ -121,6 +124,7 @@ CONFIG_USB_STORAGE=y
>  CONFIG_EDAC=y
>  CONFIG_EDAC_MM_EDAC=y
>  CONFIG_RTC_CLASS=y
> +CONFIG_RTC_DRV_DS1307=y
>  CONFIG_RTC_DRV_CMOS=y
>  CONFIG_DMADEVICES=y
>  CONFIG_FSL_DMA=y

This isn't a p1023rds.  Having one defconfig to support both boards is
good, but the defconfig should be renamed to p1023_defconfig.

> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index efdd37c..d0e8ff9 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -111,6 +111,12 @@ config P1022_RDK
>  	  This option enables support for the Freescale / iVeia P1022RDK
>  	  reference board.
>  
> +config P1023_RDB
> +	bool "Freescale P1023 RDB"
> +	select P1023_RDS
> +	help
> +	  This option enables support for the P1023 RDB board
> +

Why do you need a new kconfig option, if you're supporting both boards
with the same C file?  Just update the name and description of the
existing kconfig symbol.

-Scott