On Tue, Feb 04, 2025 at 10:31:49AM +0100, Johan Hovold wrote: > On Mon, Feb 03, 2025 at 05:16:50PM +0200, Dmitry Baryshkov wrote: > > On Mon, Feb 03, 2025 at 09:22:51AM +0100, Johan Hovold wrote: > > > On Fri, Jan 31, 2025 at 10:44:28AM -0800, Abhinav Kumar wrote: > > > > > Solution: If the user prefers iris driver and iris driver has not probed > > > > yet, and if venus tries to probe ahead of iris we keep -EDEFERing till > > > > iris probes and succeeds. Vice-versa when the preference is venus as well. > > > > > > This sounds wrong too. > > > > > > Look, first you guys need to explain *why* you want to have two drivers > > > for the same hardware (not just to me, in the commit message and cover > > > letter). > > > > > > That's something that really should never be the case and would need to > > > be motivated properly. > > > > I think it has been written several time (not sure about this commit): > > to provide a way for a migration path _while_ people are working on Iris > > features. Being able to A/B test Venus and Iris drivers and to report > > possible regressions or lack of those on the common platforms supported > > by those (sm8250 at this moment). > > You don't need a module parameter for that. > > And we're still waiting to hear the answers to all of Hans' questions. I > still haven't seen anyone explaining why any of this is needed. You > could have just continued letting Venus support 8250 so presumably there > is some benefit in using Iris instead. Which? And is that potential > benefit important enough to not just wait until Iris is up to par > feature wise? Because that's exactly opposite of what Iris developers are trying to do: SM8250 and SM8550 belong to two different generations of the FW interface. By supporting both of them in the Iris driver the developers can verify that the internal driver abstractions are good enough. It has been discussed in one of the threads (or in telcos) related to the first iterations of this driver. We definitely don't want to end up in Venus situation, where the abstractions were added afterwards and in some cases they are not the best ones. The plan is to use sm8250 as a "bridge" between two drivers, verifying that they are on par during development, then drop sm8250 from Venus driver. Then move sc7280 support too, drop all HFD_VERSION_6XX support from Venus, cleanup the remnants. I think most of that information should have been a part of the main Iris series. If it is not, please comment there, so that those commit messages can be updated. > > I'm sure you have some answers, but you need to provide them as part of > the series. > > > > Second, if the reasons for keeping both drivers are deemed justifiable, > > > you need to come up with mechanism for only binding one of them. > > > > > > I already told you that module parameters is not the way to go here (and > > > the msm drm driver's abuse of module parameters is not a good precedent > > > here). > > > > > > If this is a transitional thing (which it must be), then just add a > > > Kconfig symbol to determine which driver should probe. That's good > > > enough for evaluating whatever needs to be evaluated, and doesn't > > > depend on adding anti-patterns like module parameters (and helper > > > modules for them). > > > > No, Kconfig complicates A/B testing. What you usually want to do is to > > boot a kernel, then go over a bunch of files testing that they work with > > both drivers. Kconfig implies booting abother kernel, etc. > > No, I'm pretty sure you'd even want to reboot in between so as not to > rely on state left behind by the other driver. As a quick test I'd definitely do not want to reboot. Both drivers completely shut down the firmware running on the core, so there is no intermediate state left between driver probes. > Unbinding and rebinding drivers is not part of any normal work flow > expect possibly during development. And a developer can easily compare > Venus and Iris for 8250 without a module parameter too. Yes, we are talking about development. And yes, modparam helps. If you'd like to do two separate kernel builds, that's fine. -- With best wishes Dmitry
