Re: [Buildroot] [PATCH v2 1/1] configs/imx6ulz_bsh_smm_m2_defconfig: new defconfig

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Dario,

On Thu, 14 Dec 2023 17:18:54 +0100
Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote:

> From: Michael Trimarchi <michael@amarulasolutions.com>
> 
> BSH SMM M2 (SystemMaster M2) Board is an add-on board which provides
> input and output interfaces to a dedicated carrier board. It is designed
> mainly to provide graphical/video and connectivity interfaces to the
> appliance.
> 
> Board support package includes the following components:
> - mainline Linux kernel 6.1.68
> - mainline U-Boot 2023.10
> 
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

I am sorry to provide this feedback so late, but there is one thing to
improve in this submission:

>  board/bsh/imx6ulz-bsh-smm-m2/linux.config  | 415 +++++++++++++++++++++

This defconfig is too long and clearly contains stuff that is useless.
Could you review it and clean it up?


> +CONFIG_WL12XX=m
> +CONFIG_WL18XX=m

Need both of these?

> +CONFIG_WLCORE_SDIO=m
> +# CONFIG_WILINK_PLATFORM_DATA is not set
> +CONFIG_INPUT_EVDEV=y
> +CONFIG_INPUT_EVBUG=m
> +CONFIG_KEYBOARD_GPIO=y
> +CONFIG_KEYBOARD_SNVS_PWRKEY=y
> +CONFIG_KEYBOARD_IMX=y
> +CONFIG_MOUSE_PS2=m
> +CONFIG_MOUSE_PS2_ELANTECH=y
> +CONFIG_INPUT_TOUCHSCREEN=y
> +CONFIG_TOUCHSCREEN_ADS7846=y
> +CONFIG_TOUCHSCREEN_AD7879=y
> +CONFIG_TOUCHSCREEN_AD7879_I2C=y
> +CONFIG_TOUCHSCREEN_ATMEL_MXT=y
> +CONFIG_TOUCHSCREEN_DA9052=y
> +CONFIG_TOUCHSCREEN_EGALAX=y
> +CONFIG_TOUCHSCREEN_GOODIX=y
> +CONFIG_TOUCHSCREEN_ILI210X=y
> +CONFIG_TOUCHSCREEN_MAX11801=y
> +CONFIG_TOUCHSCREEN_IMX6UL_TSC=y
> +CONFIG_TOUCHSCREEN_MC13783=y
> +CONFIG_TOUCHSCREEN_TSC2004=y
> +CONFIG_TOUCHSCREEN_TSC2007=y
> +CONFIG_TOUCHSCREEN_STMPE=y
> +CONFIG_TOUCHSCREEN_SX8654=y

So many touchscreen drivers?

> +CONFIG_SPI=y
> +CONFIG_SPI_FSL_QUADSPI=y
> +CONFIG_SPI_GPIO=y
> +CONFIG_SPI_IMX=y
> +CONFIG_SPI_SPIDEV=y
> +CONFIG_SPI_SLAVE=y

You're using slave SPI?

> +CONFIG_SPI_SLAVE_TIME=y
> +CONFIG_SPI_SLAVE_SYSTEM_CONTROL=y
> +CONFIG_PTP_1588_CLOCK=y
> +CONFIG_GPIO_SYSFS=y
> +CONFIG_GPIO_MXC=y
> +CONFIG_GPIO_MAX732X=y
> +CONFIG_GPIO_PCA953X=y
> +CONFIG_GPIO_PCF857X=y
> +CONFIG_GPIO_STMPE=y
> +CONFIG_GPIO_74X164=y

All those GPIO drivers are needed?

> +CONFIG_POWER_RESET=y
> +CONFIG_POWER_RESET_BRCMSTB=y

A reset driver for broadcom platform?

> +CONFIG_MFD_DA9052_I2C=y
> +CONFIG_MFD_DA9062=y
> +CONFIG_MFD_DA9063=y
> +CONFIG_MFD_MC13XXX_SPI=y
> +CONFIG_MFD_MC13XXX_I2C=y
> +CONFIG_MFD_RN5T618=y
> +CONFIG_MFD_STMPE=y
> +CONFIG_MFD_WM8994=y

All those MFD devices?

> +CONFIG_REGULATOR=y
> +CONFIG_REGULATOR_FIXED_VOLTAGE=y
> +CONFIG_REGULATOR_ANATOP=y
> +CONFIG_REGULATOR_DA9052=y
> +CONFIG_REGULATOR_DA9062=y
> +CONFIG_REGULATOR_DA9063=y
> +CONFIG_REGULATOR_GPIO=y
> +CONFIG_REGULATOR_LTC3676=y
> +CONFIG_REGULATOR_MC13783=y
> +CONFIG_REGULATOR_MC13892=y
> +CONFIG_REGULATOR_PFUZE100=y
> +CONFIG_REGULATOR_RN5T618=y

All those regulators?

etc. Please review globally :-)

> diff --git a/configs/imx6ulz_bsh_smm_m2_defconfig b/configs/imx6ulz_bsh_smm_m2_defconfig
> new file mode 100644
> index 000000000000..b12f36b84e65
> --- /dev/null
> +++ b/configs/imx6ulz_bsh_smm_m2_defconfig
> @@ -0,0 +1,42 @@
> +BR2_arm=y
> +BR2_cortex_a7=y
> +BR2_ARM_FPU_VFPV3=y
> +BR2_KERNEL_HEADERS_6_1=y
> +BR2_TOOLCHAIN_BUILDROOT_CXX=y

Why is C++ enabled?

> +BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y
> +BR2_TARGET_GENERIC_GETTY_PORT="ttymxc3"
> +BR2_ROOTFS_POST_BUILD_SCRIPT="board/bsh/imx6ulz-bsh-smm-m2/post-build.sh"
> +BR2_LINUX_KERNEL=y
> +BR2_LINUX_KERNEL_CUSTOM_VERSION=y
> +BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="6.1.68"
> +BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
> +BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/bsh/imx6ulz-bsh-smm-m2/linux.config"
> +BR2_LINUX_KERNEL_DTS_SUPPORT=y
> +BR2_LINUX_KERNEL_INTREE_DTS_NAME="imx6ulz-bsh-smm-m2"
> +BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
> +BR2_PACKAGE_BUSYBOX_SHOW_OTHERS=y

Why is this option enabled?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot