Re: [PATCH v2 05/11] arm64: defconfig: rebuild default configuration

[Marcel Ziswiler]

On Sat, 2022-01-15 at 18:02 +0100, Krzysztof Kozlowski wrote:
> On 14/01/2022 15:15, Marcel Ziswiler wrote:
> > From: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx>
> > 
> > Run "make defconfig; make savedefconfig" to rebuild defconfig.
> > 
> > This re-ordered the following configuration options:
> > 
> > CONFIG_BPF_JIT=y
> > CONFIG_ARM_SCMI_PROTOCOL=y
> > CONFIG_ARM_SCPI_PROTOCOL=y
> > CONFIG_RASPBERRYPI_FIRMWARE=y
> > CONFIG_INTEL_STRATIX10_SERVICE=y
> > CONFIG_INTEL_STRATIX10_RSU=m
> > CONFIG_EFI_CAPSULE_LOADER=y
> > CONFIG_IMX_SCU=y
> > CONFIG_IMX_SCU_PD=y
> > CONFIG_CAN_FLEXCAN=m
> > CONFIG_PCIE_LAYERSCAPE_GEN4=y
> > CONFIG_MTK_DEVAPC=m
> > CONFIG_SPI_CADENCE_QUADSPI=y
> > CONFIG_MDIO_BUS_MUX_MMIOREG=y
> > CONFIG_MDIO_BUS_MUX_MULTIPLEXER=y
> > CONFIG_MESON_GXL_PHY=m
> > CONFIG_QCOM_CPR=y
> > CONFIG_ROCKCHIP_IODOMAIN=y
> > CONFIG_SENSORS_ARM_SCMI=y
> > CONFIG_QORIQ_THERMAL=m
> > CONFIG_SUN8I_THERMAL=y
> > CONFIG_TEGRA_BPMP_THERMAL=m
> > CONFIG_ARM_SMC_WATCHDOG=y
> > CONFIG_VIDEO_QCOM_CAMSS=m
> > CONFIG_DRM_PANEL_BOE_TV101WUM_NL6=m
> > CONFIG_DRM_NWL_MIPI_DSI=m
> > CONFIG_DRM_LONTIUM_LT9611UXC=m
> > CONFIG_SND_SOC_IMX_AUDMIX=m
> > CONFIG_TYPEC_HD3SS3220=m
> > CONFIG_COMMON_CLK_SCMI=y
> > CONFIG_IPQ_GCC_8074=y
> > CONFIG_SM_DISPCC_8250=y
> > CONFIG_QCOM_WCNSS_CTRL=m
> > CONFIG_ARCH_R8A774A1=y
> > CONFIG_ARCH_R8A774B1=y
> > CONFIG_ARCH_R8A774C0=y
> > CONFIG_ARCH_R8A774E1=y
> > CONFIG_ARCH_R8A77995=y
> > CONFIG_ARCH_R8A77990=y
> > CONFIG_ARCH_R8A77965=y
> > CONFIG_ARCH_R8A77970=y
> > CONFIG_HISI_PMU=y
> > CONFIG_QCOM_QFPROM=y
> > CONFIG_MUX_MMIO=y
> 
> Thanks for the changes.
> The best would be to have a separate patch only for re-ordering.

Yes, from a review perspective that makes sense. Will do so in v3.

> > And dropped the following configuration options which are nowaday's
> > already enabled (resp. disabled) by default:
> > 
> > CONFIG_MEMCG_SWAP=y
> > CONFIG_SECCOMP=y
> 
> Is it? I tried now on next-20220114 and it is still user-selectable and
> not chosen by anything.

Hm, strange. I guess, it is due to this whole patch series, given its i.MX main focus, being based on Shawn's
for-next branch. Maybe for such defconfig changes it would be better to base them on something else? Not sure,
who will ultimately pull such changes. Any suggestion?

> > CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y
> > CONFIG_QCOM_SCM=y
> > # CONFIG_BT_HS is not set
> > CONFIG_FSL_MC_BUS=y
> > CONFIG_MEDIA_CONTROLLER=y
> > CONFIG_VIDEO_V4L2_SUBDEV_API=y
> > CONFIG_SND_SOC_FSL_SAI=m
> > CONFIG_USB_CONN_GPIO=m
> > CONFIG_USB_XHCI_PCI=m
> > CONFIG_MFD_CROS_EC_DEV=y
> > CONFIG_COMMON_CLK_ZYNQMP=y
> > CONFIG_SDM_GCC_845=y
> > CONFIG_SM_GCC_8150=y
> > CONFIG_SM_GCC_8250=y
> > CONFIG_SLIMBUS=m
> > CONFIG_INTERCONNECT=y
> > CONFIG_CONFIGFS_FS=y
> 
> All three above are still user-selectable, so please leave them. It is
> redundant, but there is no guarantee that something selecting a
> user-visible symbol will stop selecting it. IOW, user-visible symbols
> should be still chosen by defconfigs if they really want them.

Well, but even if they are already enabled anyway? What is the point of savedefconfig then resp. you are saying
that the commited defconfigs should not be generated using savedefconfig? That sounds rather confusing to me.

> See for example commit a2315d3aea59 ("ARM: exynos_defconfig: Restore
> debugfs support") for rationale why we need to keep them.

Okay, I see what you mean as in relation to above mentioned commit but then any change to Kconfig dependencies
can ultimately change the behaviour of previous configs unless they are complete ones (or were just lucky to
include whatever stuff that changed). But how should one now know which of them zillions of user-selectable
options should be added to such "more robust" defconfigs?

I suggest a better approach might be to have some CI which validates defconfig changes. That way one could
easily track stuff disappearing and could at that point explicitly enable it again.

> Best regards,
> Krzysztof

Cheers

Marcel