Re: [PATCH v5 10/10] ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig

[Arnd Bergmann]

On Fri, Dec 3, 2021 at 8:34 AM Qin Jian <qinjian@xxxxxxxxxxx> wrote:
>
> Add generic Sunplus SP7021 based board defconfig
>
> Signed-off-by: Qin Jian <qinjian@xxxxxxxxxxx>
> ---
>  MAINTAINERS                       |   1 +
>  arch/arm/configs/sp7021_defconfig | 178 ++++++++++++++++++++++++++++++

Most platforms don't even have a custom defconfig any more. If you
think you need this one,
I would suggest naming it sunplus_defconfig so it makes more sense
when other related
SoCs are added to it.

I would also suggest enabling your platform specific drivers in
multi_v7_defconfig,
and testing that one to ensure it works.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0ae537a41..9340f8760 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2664,6 +2664,7 @@ F:        Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
>  F:     Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
>  F:     Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
>  F:     Documentation/devicetree/bindings/reset/sunplus,reset.yaml
> +F:     arch/arm/configs/sp7021_*defconfig
>  F:     arch/arm/mach-sunplus/
>  F:     drivers/clk/clk-sp7021.c
>  F:     drivers/irqchip/irq-sp7021-intc.c
> diff --git a/arch/arm/configs/sp7021_defconfig b/arch/arm/configs/sp7021_defconfig
> new file mode 100644
> index 000000000..249fe779e
> --- /dev/null
> +++ b/arch/arm/configs/sp7021_defconfig
> @@ -0,0 +1,178 @@
> +CONFIG_DEFAULT_HOSTNAME="SP7021-Ev"

I'd leave this out of the defconfig

> +CONFIG_SYSVIPC=y
> +CONFIG_USELIB=y

You almost certainly don't need CONFIG_USELIB

> +CONFIG_BLK_DEV_INITRD=y
> +CONFIG_INITRAMFS_SOURCE="../rootfs/initramfs/disk/ ../rootfs/initramfs/initramfs.devnodes"
> +CONFIG_INITRAMFS_ROOT_UID=-1

This will cause a build failure for everyone else I think

> +CONFIG_EMBEDDED=y

Probably not needed, otherwise explain what option you need to touch
that depends on this.

> +CONFIG_PERF_EVENTS=y
> +CONFIG_SLAB=y
> +CONFIG_ARCH_SUNPLUS=y
> +# CONFIG_VDSO is not set
> +CONFIG_SMP=y
> +CONFIG_HAVE_ARM_ARCH_TIMER=y
> +CONFIG_THUMB2_KERNEL=y

Ah nice, we don't have enough defconfigs using THUMB2_KERNEL. This may mean
that you get reports about thumb2 specific bugs that nobody else runs
into, but that is
better than if nobody uses that.

> +CONFIG_HIGHMEM=y
> +# CONFIG_HIGHPTE is not set

What is the maximum amount of supported RAM? Note that HIGHMEM support
will likely
go away at some point, so it may be better to use a good VMSPLIT
setting that works with
all your machines without requiring highmem.

> +CONFIG_ZBOOT_ROM_TEXT=0x98307000
> +CONFIG_ZBOOT_ROM_BSS=0x03400000

Where do these come from? I see you don't set CONFIG_ZBOOT_ROM, so they will be
ignored, but if you do rely on these, it likely means that your
machine is unable to
run a multi_v7_defconfig kernel, which is bad.

> +CONFIG_CMDLINE="root=/dev/ram rw init=/init console=ttyS0,115200 earlyprintk mem=512M@0x0"

Remove this, the configuration should come from the bootloader.

> +CONFIG_AUTO_ZRELADDR=y

This is always enabled

> +CONFIG_CAIF=y

Are you actually using CAIF? Which driver do you use?

> +CONFIG_CEPH_LIB=y
> +CONFIG_CEPH_LIB_USE_DNS_RESOLVER=y

Same here, do you use this?

> +CONFIG_INPUT_MISC=y
> +CONFIG_INPUT_UINPUT=y
> +CONFIG_SERIO_LIBPS2=y

Do you have a physical PS/2 keyboard connector

> +CONFIG_USB_VIDEO_CLASS=y
> +CONFIG_FB=y
> +CONFIG_FB_SP7021=y

CONFIG_FB is no longer recommended for new platforms, please use
CONFIG_SIMPLEDRM for the moment, until you have converted your
driver to the DRM framework.

> +CONFIG_GEMINI_USB=y

What is this?

> +CONFIG_EXT2_FS=y

If you use eMMC storage, I recommend using EXT4_FS instead of EXT2_FS


         Arnd