lkml.org 
[lkml]   [2022]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH 0/2] arm64 defconfig: Get faddr2line working
From
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

\
 
 \ /
  Last update: 2022-07-25 16:05    [W:0.254 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site