Re: [PATCH 0/2] arm64 defconfig: Get faddr2line working

[John Garry]

On 25/07/2022 13:51, Arnd Bergmann wrote:
> >   From commit fdf4632aa834, we enable config ATH11K_PCI, which selects

That should have been 231a136fdf4632a - I chopped off the leading 
characters by accident.

> > config QRTR, so no need to explicitly enable in the defconfig.
> Something is wrong here, as qrtr is used the same way in both ath10k and
> ath11k, but only one of them selects the symbol. My guess is that there is
> no actual hard requirement to enable it.

I'm not sure. From commit 1399fb87ea3e ("ath11k: register MHI controller 
device for QCA6390"), QRTR/MHI bus seems to be used for ath11k only:

Extract from that commit:

diff --git a/drivers/net/wireless/ath/ath11k/Kconfig 
b/drivers/net/wireless/ath/ath11k/Kconfig
index 2a792ddd6fea..7e5094e0e7bb 100644
--- a/drivers/net/wireless/ath/ath11k/Kconfig
+++ b/drivers/net/wireless/ath/ath11k/Kconfig
@@ -21,6 +21,9 @@ config ATH11K_AHB
config ATH11K_PCI
       tristate "Atheros ath11k PCI support"
       depends on ATH11K && PCI
+       select MHI_BUS
+       select QRTR
+       select QRTR_MHI


> > > CONFIG_PINCTRL_MSM=y
> >   From commit c807a335d3b1, config PINCTRL_SM8450 is enabled, which
> > selects PINCTRL_MSM, so no need to explicitly enable in the defconfig.
> This looks like a bug, where the driver should have used 'depends on'
> rather than 'select'.

ok, I see that this is the odd one out in that kconfig as the other 
configs "depends on", like you say

> > > CONFIG_SND_SOC_TEGRA210_OPE=m
> > There is no issue on next-20220722.
> >
> > On arm-soc arm/defconfig, config SND_SOC_TEGRA210_OPE just does not yet
> > exist, so that's why it get removed from the sync of the defconfig.
> Ok
>
> > > CONFIG_MAILBOX=y
> > In commit fc739069aa92, config MAILBOX was enabled in the defconfig but
> > it was already being enabled from elsewhere. There was definitely no
> > sync of the savedefconfig going on there:)
> I see it's selected by a couple of drivers using mailboxes, and I
> agree we shouldn't
> need it here. It might be good to just hide the symbol in this case
> and select it
> consistently from all drivers using it.

Uhh, we have ~15x "select" and ~18x "depends on"...

> > > CONFIG_QCOM_ICC_BWMON=m
> > Commit 76f11e77f919 enabled config QCOM_ICC_BWMON, but like config
> > ARCH_BCMBCA, that config does not exist on arm-soc arm/defconfig branch
> >
> > On next-20220722, no sync is required
> ok
>
> > > CONFIG_SLIMBUS=m
> > Config 5bd773242f75 added a kconfig "imply" on config SLIMBUS from
> > config SOUNDWIRE_QCOM, and this happens to mean that we don't explicitly
> > require config SLIMBUS enabled in the defconfig.
> That 'imply' looks like it was added to solve a problem using the old 'imply'
> semantics that are not what this keyword does today.

I didn't even know it existed.

> I suspect it's still
> broken when CONFIG=SOUNDWIRE_QCOM=y and CONFIG_SLIBMUS=m,
> and the correct fix is to use a dependency like
>
>        depends on SLIMBUS || !SLIMBUS
>
> so the defconfig symbol should stay.

Let me see if I can break it and then fix it.

> > > CONFIG_INTERCONNECT=y
> > Since commit 06f079816d4c, config TEGRA_MC added a kconfig select on
> > config INTERCONNECT, so no need to explicitly enable from the kconfig
> We have one driver using 'depends on INTERCONNECT' and two drivers
> using 'select INTERCONNECT', so at least one of them is wrong.

INTERCONNECT has no dependencies, so "select" - like for MAILBOX - 
should be fine, I suppose

> There are also a couple of drivers that use neither and instead rely on the
> IS_ENABLED() check in include/linux/interconnect.h, but this is potentially
> wrong in the same way as the SLIMBUS dependency above.
>
> This clearly needs some more discussion. I would suggest removing the
> #iff check in the header and forcing all users to use 'depends on', leaving
> the defconfig symbol.
>
> > > CONFIG_CONFIGFS_FS=y
> >   From commit cd8bc7d4eb66, config PCI_ENDPOINT_CONFIGFS is enabled in
> > the defconfig, and this selects CONFIG_CONFIGFS_FS, so no need to have
> > explicit enabling in the defconfig.
> Again, half the users of CONFIGFS_FS use 'depends on', the other half use
> 'select'. We should be consistent here and always use the same method,
> probably 'depends on' if we want to keep it as user-visible.

I don't know ....

> > We still have issues on next-22072022:
> > CONFIG_ARM_CPUIDE
> > CONFIG_DEBUG_INFO
> > CONFIG_DEBUG_INFO_REDUCED
> >
> > The latter two are not an issue on the arm-soc arm/defconfig.
> Ok.
>
> > So let me know if any comments or how to proceed.
> >
> > And would each config item deletion merit a separate patch?
> You send a combined patch for the obvious ones (secccomp
> and mailbox AFAICT) or send them separately. For the other ones I think
> we should try fixing the Kconfig files first, otherwise we just end up
> putting them back afterwards.

ok, fine. I'll deal with the obvious changes first plus 
CONFIG_DEBUG_INFO and then the non-obvious, non-trivial ones. I'll base 
on your arm/defconfig branch (for defconfig changes).

Thanks,
John