Re: [PATCH] arm64: dts: qcom: sdm845-db845c: Move LVS regulator nodes up

[Krzysztof Kozlowski]

On 16/06/2023 19:09, Amit Pundir wrote:
> Hi,
> 
> On Fri, 16 Jun 2023 at 13:57, Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
>>
>>
>> So you have interconnect as module - this is not a supported setup. It
>> might work with if all the modules are loaded very early or might not.
>> Pinctrl is another driver which should be built-in.
>>
>> With your defconfig I see regular issue - console and system dies
>> because of lack of interconnects, most likely. I don't see your WARNs -
>> I just see usual hang.
>>
>> See:
>> https://lore.kernel.org/all/20221021032702.1340963-1-krzysztof.kozlowski@xxxxxxxxxx/
>>
>> If you want them to really be modules, then you need to fix all the
>> dependencies (SOFTDEP?), probe ordering glitches. It's not a problem of
>> DTS. Just because something can be built as module, does not mean it
>> will work. We don't test it, we don't work with them as modules.
> 
> I do somewhat agree with most of your arguments but not this one. If a
> driver doesn't work as a module then it shouldn't be allowed to build
> as a module. 

Of course you are right. That's why I am pushing against blindly adding
"tristate" by everyone working on GKI. Because such folks like to make
them tristate, but not actually test it or work on issues later.

That's exactly the case from Google and Samsung patches here:
https://lore.kernel.org/all/ac328b6a-a8e2-873d-4015-814cb4f5588e@xxxxxxxxxxxxx/
and in previous submissions.

> I took a quick look at the history of the interconnect
> driver and it is tristate from the beginning. And not converted to a
> modular build later-on like some of the other drivers to support GKI.

OK, maybe it was never actually tested. Or maybe some versions were
working on boards where debug serial does not have interconnect, but new
chips just followed the pattern without testing?
> 
>>
>> It's kind of the same as here:
>> https://lore.kernel.org/all/ac328b6a-a8e2-873d-4015-814cb4f5588e@xxxxxxxxxxxxx/
>>
>> I understand that we might have here regression, if these were working
>> as modules, but I don't think we ever really committed to it. We can as
>> well make it non-module to solve the regression.
> 
> Sure. But since v6.4 is around the corner, can we merge this
> workaround for now, while a proper fix is being worked upon.

DTS workaround? No. I don't agree. Once it is merged it will not be fixed.

I am perfectly fine though with making the interconnect or even rpmh
regulator bool instead of tristate.

Best regards,
Krzysztof