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

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

On 28/07/2022 09:23, Arnd Bergmann wrote:
> On Thu, Jul 28, 2022 at 10:06 AM John Garry <john.garry@huawei.com> 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