Re: [PATCH 2/4] irqchip: Add IRQCHIP_DECLARE_DRIVER macro

[Marc Zyngier <marc.zyngier@arm.com>]

On 27/01/17 18:13, Andrey Smirnov wrote:
> +Cc linux-kernel@vger.kernel.org since I messed up original distribution list
> 
> On Fri, Jan 27, 2017 at 12:51 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 26/01/17 21:56, Andrey Smirnov wrote:
>>> Add IRQCHIP_DECLARE_DRIVER macro to allow having driver code that both
>>> registers irqchip and a platform driver. Based on analogous code of
>>> CLK_OF_DECLARE_DRIVER.
>>>
>>> Cc: yurovsky@gmail.com
>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Jason Cooper <jason@lakedaemon.net>
>>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>>> ---
>>>  include/linux/irqchip.h | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
>>> index 89c34b2..611e8cc 100644
>>> --- a/include/linux/irqchip.h
>>> +++ b/include/linux/irqchip.h
>>> @@ -27,6 +27,22 @@
>>>  #define IRQCHIP_DECLARE(name, compat, fn) OF_DECLARE_2(irqchip, name, compat, fn)
>>>
>>>  /*
>>> + * Use this macro when you have a driver that requires two
>>> + * initialization routines, one at IRQCHIP_DECLARE, and one at
>>> + * platform device probe
>>> + */
>>> +#define IRQCHIP_DECLARE_DRIVER(name, compat, fn)                     \
>>> +     static int __init                                               \
>>> +     name##_of_irqchip_init_driver(struct device_node *np,           \
>>> +                                   struct device_node *parent)       \
>>> +     {                                                               \
>>> +             of_node_clear_flag(np, OF_POPULATED);                   \
>>> +             return fn(np, parent);                                  \
>>> +     }                                                               \
>>> +     OF_DECLARE_2(irqchip, name, compat, name##_of_irqchip_init_driver)
>>> +
>>> +
>>> +/*
>>>   * This macro must be used by the different irqchip drivers to declare
>>>   * the association between their version and their initialization function.
>>>   *
>>>
>>
>> Why do we need this new macro? What problem does it solve that would not
>> be solved by having proper dependency tracking?
> 
> The problem that I was trying to solve was trying to associate a
> platform device with a DT-node that had a IRQCHIP_DECLARE declared for
> it. After a bit of digging around the codebase I found
> CLK_OF_DECLARE_DRIVER which seemed to exist to solve exactly the same
> kind of problem I was having so I took it as "inspiration" and created
> IRQCHIP_DECLARE_DRIVER.

Irk. I see.

> I am not sure what you mean by "proper dependency tracking"(most
> likely due to my ignorance), can you point me to a concrete example or
> explain what you mean a bit more?

Well, what I was angling at is that this whole mess is actually created
by a number of "things" (clocks, timers and interrupt controllers) not
being represented as first class devices. That's mostly because they are
probed before the device model is up and running.

Now, you (and apparently many others before you) are playing tricks on
the OF layer to allow things to be probed as a device on top of using
the pre-device match.

While I really dislike it, I wouldn't object if this was universally
applicable to all firmware interfaces. But that's the point where things
break. This is completely tied to the device-tree (I have no particular
sympathies for ACPI, but I do have to deal with it).

I'd rather work on untangling the early boot so that we can have devices
be probed as early as we need, and get rid of the current hacks.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...