Re: [PATCH v2 4/8] irqchip: armada-370-xp: implement MSI support

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

Dear Thierry Reding,

On Tue, 18 Jun 2013 13:26:04 +0200, Thierry Reding wrote:

> > As I replied to Grant, of_find_device_by_node() returns NULL, I believe
> > because the all IRQ controller driver initialization is done pretty
> > early, before the of_platform_populate() call is made, so there is no
> > platform_device associated with the IRQ controller node at the time the
> > armada_370_xp_mpic_of_init() function is called.
> > 
> > Do you see another approach, especially in relation to your comment on
> > PATCH 2/8 ?
> 
> Hmmm, that's too bad. The only other possibility that I see is that you
> could associate the struct device at a later point when it becomes
> available, but looking at the irqchip driver it doesn't look like you
> get notification of that either. I suppose you could add a
> platform_driver to it and hook things up in its .probe() callback, but
> I'm not sure if that's in line with how the irqchip was designed. Adding
> Grant Likely and Thomas Gleixner on Cc, maybe they have better advice.

If we do hook the MSI stuff in a ->probe() callback, then we'd have a
dependency between the ->probe() of the PCIe driver and the ->probe()
of the IRQ controller driver. In order for the PCIe ->probe() to
succeed, it needs the MSI controller to be registered, which wouldn't
appear until the ->probe() of the IRQ controller driver gets called. A
typical case of platform device dependency where the two platform
devices don't have a bus -> child dependency. Could be handled by a
-EPROBE_DEFER trick, though.

> Looking at other irqchip drivers I find it a bit odd to see how they're
> structured, though. We've been preaching for years that drivers should
> be well-encapsulated and told everybody it was bad to use globals and
> they should be associating driver-specific data with each instance of a
> device. Then comes along irqchip and all of a sudden it's okay to use
> globals again. It feels a bit fragile.

Yes, I've seen this as well, and I'm thinking of doing some
improvements in this area if there's some interest. But I believe this
is fairly separate from the specific discussion of this patch set.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com