On Fri, Oct 4, 2024 at 7:31 PM Bjorn Andersson <andersson@xxxxxxxxxx> wrote: > > > > > + /* > > + * Old device trees for some platforms already define wifi nodes for > > + * the WCN family of chips since before power sequencing was added > > + * upstream. > > + * > > + * These nodes don't consume the regulator outputs from the PMU and > > + * if we allow this driver to bind to one of such "incomplete" nodes, > > + * we'll see a kernel log error about the indefinite probe deferral. > > + * > > + * Let's check the existence of the regulator supply that exists on all > > + * WCN models before moving forward. > > + * > > + * NOTE: If this driver is ever used to support a device other than > > + * a WCN chip, the following lines should become conditional and depend > > + * on the compatible string. > > What do you mean "is ever used ... other than WCN chip"? > This driver was released as part of v6.11 and so far (until v6.12) is only used to support the WCN chips. That's not to say that it cannot be extended to support more hardware. I don't know how to put it in simpler words. > This driver and the power sequence framework was presented as a > completely generic solution to solve all kinds of PCI power sequence > problems - upon which the WCN case was built. > I never presented anything as "completely generic". You demanded that I make it into a miraculous catch-all solution. I argued that there's no such thing and this kind of attitude is precisely why it's so hard to get anything done in the kernel. I made it *generic enough* and we can cross any bridge requiring new features when we get to it. This is why we have no stable APIs in the kernel! And why every long-lived user-space library is at major API version 2 or 3. You can never possibly get *everything* right. Also: there's a big difference between the framework and this driver. A driver is just a consumer of the larger framework. We could possibly make it WCN-specific and create a new one for QPS615 (even if it was to use pwrseq as well) instead of cramming a solution for every possible corner case into a single compilation unit. > In fact, if I read this correctly, the second user of the power sequence > implementation (the QPS615, proposed in [1]), would break if this check > is added. > Is it queued for v6.13 yet? If not, then we make no guarantees. A regression is when something upstream stops working, not when yet-unmerged patches from the list do. Have you really never had to modify existing code to accommodate new one? This is a fix that needs to go into v6.12 and be backported to v6.11. Hence a simple patch. For v6.13 we can easily extend the match data to become a structure indicating whether we should do the check or not. That's a really simple change too. But it would grow the fix needlessly. > Add to this that your colleagues are pushing people to implement simple > power supplies for M.2-connected devices into this framework - which I > can only assume would trip on this as well (the one supply pin in a M.2. > connector isn't named "vddaon"). > > [1] https://lore.kernel.org/all/20240803-qps615-v2-3-9560b7c71369@xxxxxxxxxxx/ > I'm not sure what exactly you're referring to here. Bart
