On Tue, Jul 16, 2024 at 10:29 PM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, 16 Jul 2024 at 13:10, Bartosz Golaszewski > <bartosz.golaszewski@xxxxxxxxxx> wrote: > > > > select PCI_PWRCTL_PWRSEQ if ARCH_QCOM > > > > in the ATH11K_PCI and ATH12K Kconfig entries? Am I getting this right? > > So on the whole, I'd prefer these things to be done where they are > actually required. > > But I'm not actually entirely sure what the hard _requirements_ from a > hardware - or a kernel build - standpoint actually are. > > If there aren't any hard requirements at all, then maybe the whole "do > you want pweseq" should be an actual question (but limited only to > situations where it makes sense). > > If the hard requirement is at the level of the driver itself, then the > "select" should be in the driver. > > That doesn't seem to be the case here, since ATH11K_PCI certainly > works without it, but if that driver requires power sequencing on > ARCH_QCOM platforms, then maybe that is indeed the right thing. > > And if the hard requirement comes from some SoC setup, then optimally > I think the "select" should be at that level, but we don't actually > seem to have that level of questions (but maybe something in > > drivers/soc/qcom/Kconfig > > might make sense?) > The hard requirement really comes from the board level - not SoC. It's the board that has the BT/WLAN module hardwired and - depending on how the module is powered - may or may not require power sequencing. But I don't think we have any per-board infrastructure in Kconfig. > Anyway, this is not necessarily something where there is "one correct > answer". This may be a somewhat gray area, and it looks like ARCH_QCOM > is a very big "any Qualcomm SoC" thing and not very specific. > > So I'm not sure what the right answer is, but I *am* pretty sure of > what the wrong answer is. And this: > > default m if ((ATH11K_PCI || ATH12K) && ARCH_QCOM) > > looks like it cannot possibly be right if ATH11K_PCI is built-in, > since then the probing of the device will happen long before that > PCI_PWRCTL_PWRSEQ module has been loaded. > > And that doesn't sound sensible to me. Does it? > > TL;DR: I do think that the > > select PCI_PWRCTL_PWRSEQ if ARCH_QCOM > > in the ATH11K_PCI and ATH12K Kconfig entries *may* be the right thing. > But again, I'm not actually 100% sure of the hard requirements here. > > Linus After sleeping on it I really think that it may be better to introduce a more generic ARCH_HAVE_PCI_PWRCTL symbol so that we don't have to update the ATH Kconfig entires for every new platform that needs it. For want of a more fine-grained solution, we would select it from ARCH_QCOM. Bart