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

[Arnd Bergmann]

On Thu, Jul 28, 2022 at 10:06 AM John Garry <john.garry@xxxxxxxxxx> wrote:
>
> trim list

Adding a few other people for slimbus.

> On 25/07/2022 13:51, Arnd Bergmann wrote:
> >>> 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 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.
> >
>
> JFYI, building for CONFIG_SOUNDWIRE_QCOM=y and CONFIG_SLIBMUS=m is ok.
> The driver has a runtime check for CONFIG_SLIMBUS in qcom_swrm_probe().
>
> That implementation seems consistent with guidance in
> kconfig-language.rst - so do you think that there is still a problem?

I see Vinod added the IS_REACHABLE() check, which makes it build, but
I think both the 'imply' and the IS_REACHABLE() are mistakes here, as they
are almost everywhere else.

>From what I understand, the driver can actually use two different
back-ends, with slimbus being one of them. In the configuration
with CONFIG_SOUNDWIRE_QCOM=y and CONFIG_SLIBMUS=m,
I think you can end up with a slimbus device being detected at
runtime, which gets passed to the built-in qcom driver, but that driver
then misdetects the bus because it is not linked against the
slimbus module. It then uses the incorrect code path, causing
unintended behavior.

The 'depends on SLIMBUS || !SLIMBUS' dependency would completely
avoid this issue and make the driver work correctly in each configuration.

        Arnd