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

[Srinivas Kandagatla]

On 28/07/2022 09:23, Arnd Bergmann wrote:
> 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,

w.r.t Qualcomm SoundWire Controller.

There are two types of integrations at IP level.
1> SoundWire Controller is part of Codec which is connected over 
SlimBus. ex: SoCs SDM845 and older ones
	All the read writes have to go via SlimBus physical layer.

2> SoundWire Controller is part of Codec which is MMIO,ex: SM8250 and 
newer SoCs. No SlimBus dependency.

As we are using a common driver for this, we wanted to check if the 
parent was a slimbus or not, to be able to select correct read/write 
interfaces.

rethinking about this approach, It should be doable to also derive this 
information from compatible strings, Soundwire controller versions 1.3.0 
and below are always of type 1 and the newer ones are type 2.

So this could get rid of IS_REACHABLE check along with references to 
slimbus_bus from driver.

expressing the SlimBus dependency for type 1 should be either captured 
using imply SLIMBUS or depends in Kconfig.


> 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


Thanks,
Srini