Re: [PATCH v5 2/5] driver core: Functional dependencies tracking support

[Greg Kroah-Hartman]

On Fri, Nov 11, 2016 at 02:08:35AM +0200, Laurent Pinchart wrote:
> Hi Greg,
> 
> On Thursday 10 Nov 2016 23:40:54 Greg Kroah-Hartman wrote:
> > On Thu, Nov 10, 2016 at 11:04:07PM +0100, Luis R. Rodriguez wrote:
> > > This is more forward looking, but -- if we had an annotation in
> > > Kconfig/turned to a mod info section, or to start off with just a driver
> > > MODULE_SUGGESTS() macro to start off with it might suffice for the driver
> > > core to request_module() annotated dependencies, such requests could be
> > > explicitly suggested as synchronous so init + probe do run together
> > > (as-is today), after which it could know that all possible drivers that
> > > needed to be loaded should now be loaded. If this sounds plausible to
> > > help, do we have drivers where we can test this on? For instance, since
> > > the functional dependency framework annotates functional dependencies for
> > > consumers/providers for suspend/resume and un time PM could such
> > > MODULE_SUGGESTS() annotations be considered on the consumers to suggest
> > > the provider drivers so their own probe yields to their providers to try
> > > first ?
> > 
> > No.
> > 
> > Stop.
> > 
> > First off, the "driver core" NEVER can "know" if "all possible drivers
> > that should be loaded, are loaded.  That way lies madness and
> > impossibility.
> > 
> > Secondly, yet-another-section isn't going to help anything here, we
> > alredy "suggest" to userspace a bunch of stuff, so we get the needed
> > modules loaded, at sometime in the future, if they are around, and
> > userspace feels like it.  That's the best we can ever do.
> > 
> > Don't try to make this more difficult than it is please. DEFER works
> > today really really well, and it's really really simple.
> > Inter-dependancy of modules and devices connected to each other are two
> > different things, be careful about this.
> 
> One issue we don't address today is handling of optional dependencies. A 
> simple example is an SPI controller that can use a DMA engine or work in PIO 
> mode. At probe time the driver will request a DMA channel if the platform 
> (ACPI, DT, platform data) specifies that DMA is available. This can fail for 
> various reasons, one of them being that the DMA engine driver hasn't probed 
> the DMA device yet. In that case the SPI controller driver will continue in 
> PIO mode, ignoring the DMA engine that will later be probed. We can't defer 
> probing of the SPI controller as the DMA engine driver might never get loaded, 
> which would result in the SPI controller probe being deferred forever.
> 
> One solution for this type of dependency issue would be to notify the SPI 
> controller driver after probe that the DMA channel is now available. I'd like 
> to avoid that though, as it would drastically increase the complexity of lots 
> of drivers and create lots of race conditions.
> 
> There are certain configurations that we could possibly consider as invalid. 
> For instance if the SPI controller driver is built-in and the DMA engine 
> driver built as a module, the user clearly shot themselves in the foot and the 
> kernel can't be blamed.
> 
> For resources that can't be built as a module (IOMMUs for instance) we thus 
> only have to consider the case where both drivers are built-in, as the 
> resource built-in and consumer as a module should work properly from an 
> ordering point of view (at least as long as we don't allow asynchronous 
> probing of built-in drivers to be delayed enough for modules to be loaded...). 
> In this case, if the resource driver isn't available when the consumer is 
> probed, if will never be available at the consumer can safely proceed in a 
> degraded mode. We would thus only need to solve the probe ordering issue.
> 
> I'm not sure how far these simple(r) solutions that consider certain cases as 
> invalid would scale though, and whether we won't need a more generic solution 
> at some point anyway.

I would love to see a generic solution that works for all of these
complex cases, as I agree with you, it's complex :)

But I have yet to see any such patches that implement this.  As always,
I am very glad to review anything that people create, but I don't have
the time to work on such a solution myself at the moment.

thanks,

greg k-h