On Sat, 17 Jun 2023 at 12:51, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > 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. As Doug also mentioned in one of his earlier emails, this workaround is only limited to one particular board. If I try to change the common interconnect and/or rpmh driver then it will need ack from other stake holders as well and I'll most likely get more pushback from that side. Regards, Amit Pundir
