Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190905081140.1428-1-james.tai@realtek.com>
In-Reply-To: <20190905081140.1428-1-james.tai@realtek.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Thu, 5 Sep 2019 10:46:25 +0200
Message-ID: <CAK8P3a2j2m0mUPLDe5G19Xzu2t+xbO4QWvg=PdQWnGoqTkDpsg@mail.gmail.com>
Subject: Re: [PATCH] ARM: config: Add Realtek RTD16XX defconfig
To:     jamestai.sky@gmail.com
Cc:     Linux ARM <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        CY_Huang <cy.huang@realtek.com>,
        "james.tai" <james.tai@realtek.com>,
        Russell King <linux@armlinux.org.uk>,
        Phinex Hung <phinex@realtek.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Sep 5, 2019 at 10:14 AM <jamestai.sky@gmail.com> wrote:
>
> From: "james.tai" <james.tai@realtek.com>
>
> Add a defconfig for Realtek RTD16XX platform.
>
> Signed-off-by: james.tai <james.tai@realtek.com>
> ---
>  arch/arm/configs/rtd16xx_defconfig | 427 +++++++++++++++++++++++++++++

We usually try to have one defconfig per vendor. Expecting that
there will be other Realtek SoCs in the future that we may add
here, I would name this 'rtd_defconfig' or 'realtek_defconfig'.

Please also add the set of options you want to multi_v7_defconfig
so you are able to boot with that.

>  1 file changed, 427 insertions(+)
>  create mode 100644 arch/arm/configs/rtd16xx_defconfig
>
> diff --git a/arch/arm/configs/rtd16xx_defconfig b/arch/arm/configs/rtd16xx_defconfig
> new file mode 100644
> index 000000000000..49bcbe6c6af8
> --- /dev/null
> +++ b/arch/arm/configs/rtd16xx_defconfig
> @@ -0,0 +1,427 @@
> +CONFIG_SYSVIPC=y
> +CONFIG_NO_HZ=y
> +CONFIG_HIGH_RES_TIMERS=y
> +CONFIG_CGROUPS=y
> +CONFIG_BLK_DEV_INITRD=y
> +CONFIG_EMBEDDED=y

I normally would not turn on CONFIG_EMBEDDED, this is only
needed to change some rare options.

> +CONFIG_PERF_EVENTS=y
> +CONFIG_ARCH_REALTEK=y
> +CONFIG_ARCH_RTD16XX=y
> +CONFIG_ARM_THUMBEE=y

ThumbEE is deprecated in ARMv8, and one usually should not
rely on it. If you don't actually need it, just turn it off.

(note: this is unrelated to regular thumb execution, which
is enabled by default)

> +# CONFIG_CACHE_L2X0 is not set
> +# CONFIG_ARM_ERRATA_643719 is not set
> +CONFIG_ARM_ERRATA_814220=y
> +CONFIG_SMP=y
> +CONFIG_SCHED_MC=y
> +CONFIG_SCHED_SMT=y

If you don't have SMT in the CPU, there is no need ot enable this.

> +CONFIG_HAVE_ARM_ARCH_TIMER=y
> +CONFIG_MCPM=y
> +CONFIG_NR_CPUS=6
> +CONFIG_HZ_250=y
> +CONFIG_OABI_COMPAT=y

It seems unlikely you want OABI_COMPAT

> +CONFIG_HIGHMEM=y
> +CONFIG_FORCE_MAX_ZONEORDER=12
> +CONFIG_SECCOMP=y
> +CONFIG_ARM_APPENDED_DTB=y
> +CONFIG_ARM_ATAG_DTB_COMPAT=y
> +CONFIG_KEXEC=y
> +CONFIG_EFI=y

What method do you actually use for booting? New platforms
should generally not require CONFIG_ARM_APPENDED_DTB
or CONFIG_ARM_ATAG_DTB_COMPAT, and I suspect you
don't use EFI.

> +CONFIG_CPUFREQ_DT=y
> +CONFIG_QORIQ_CPUFREQ=y

QORIQ_CPUFREQ is a platform specific option that you
won't need.

> +CONFIG_NET_DSA=m
> +CONFIG_CAN=y
> +CONFIG_CAN_FLEXCAN=m
> +CONFIG_CAN_RCAR=m
> +CONFIG_BT=m
> +CONFIG_BT_HCIUART=m
> +CONFIG_BT_HCIUART_BCM=y
> +CONFIG_BT_MRVL=m
> +CONFIG_BT_MRVL_SDIO=m

Many more hardware specific drivers here that you should turn off

> +CONFIG_MTD=y
> +CONFIG_MTD_CMDLINE_PARTS=y
> +CONFIG_MTD_BLOCK=y
> +CONFIG_MTD_CFI=y
> +CONFIG_MTD_PHYSMAP=y
> +CONFIG_MTD_PHYSMAP_OF=y
> +CONFIG_MTD_RAW_NAND=y
> +CONFIG_MTD_NAND_DENALI_DT=y
> +CONFIG_MTD_NAND_BRCMNAND=y

and here.

> +CONFIG_BLK_DEV_LOOP=y
> +CONFIG_BLK_DEV_RAM=y
> +CONFIG_BLK_DEV_RAM_SIZE=65536

Do you require BLK_DEV_RAM for initrd? Normally one uses
initramfs instead or tmpfs instead.

> +# CONFIG_NET_VENDOR_3COM is not set
> +# CONFIG_NET_VENDOR_ADAPTEC is not set
> +# CONFIG_NET_VENDOR_AGERE is not set
> +# CONFIG_NET_VENDOR_ALACRITECH is not set
> +# CONFIG_NET_VENDOR_ALTEON is not set

I would trim the list here, just leave all network device vendors
enabled, they don't hurt.

> +CONFIG_USB_PEGASUS=y
> +CONFIG_USB_RTL8152=m
> +CONFIG_USB_LAN78XX=m
> +CONFIG_USB_USBNET=y
> +CONFIG_USB_NET_SMSC75XX=y
> +CONFIG_USB_NET_SMSC95XX=y
> +CONFIG_BRCMFMAC=m
> +CONFIG_MWIFIEX=m
> +CONFIG_MWIFIEX_SDIO=m
> +CONFIG_RT2X00=m
> +CONFIG_RT2800USB=m

Do you need all of the above? It's no problem to enable
them if you do, it just seems unusual.

        Arnd