2021-09-24 21:47:31

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 07/11] of/irq: Export of_irq_count to drivers

In order to build drivers/irqchip/irq-bcm7120-l2.c as a module, we will
need to have of_irq_count() exported to modules.

Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/of/irq.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 352e14b007e7..949b9d1f8729 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -440,6 +440,7 @@ int of_irq_count(struct device_node *dev)

return nr;
}
+EXPORT_SYMBOL_GPL(of_irq_count);

/**
* of_irq_to_resource_table - Fill in resource table with node's IRQ info
--
2.25.1


2021-09-27 19:10:42

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 07/11] of/irq: Export of_irq_count to drivers

On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <[email protected]> wrote:
>
> In order to build drivers/irqchip/irq-bcm7120-l2.c as a module, we will
> need to have of_irq_count() exported to modules.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/of/irq.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 352e14b007e7..949b9d1f8729 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -440,6 +440,7 @@ int of_irq_count(struct device_node *dev)
>
> return nr;
> }
> +EXPORT_SYMBOL_GPL(of_irq_count);

Please convert to use platform_irq_count() instead.

Rob

2021-09-27 19:30:19

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 07/11] of/irq: Export of_irq_count to drivers

On 9/27/21 12:08 PM, Rob Herring wrote:
> On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <[email protected]> wrote:
>>
>> In order to build drivers/irqchip/irq-bcm7120-l2.c as a module, we will
>> need to have of_irq_count() exported to modules.
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> drivers/of/irq.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>> index 352e14b007e7..949b9d1f8729 100644
>> --- a/drivers/of/irq.c
>> +++ b/drivers/of/irq.c
>> @@ -440,6 +440,7 @@ int of_irq_count(struct device_node *dev)
>>
>> return nr;
>> }
>> +EXPORT_SYMBOL_GPL(of_irq_count);
>
> Please convert to use platform_irq_count() instead.

That requires a platform_device to be passed to platform_irq_count(),
will that work even when the drivers remain built into the kernel and
get initialized early on?
--
Florian

2021-09-27 19:46:27

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 07/11] of/irq: Export of_irq_count to drivers

On Mon, Sep 27, 2021 at 2:28 PM Florian Fainelli <[email protected]> wrote:
>
> On 9/27/21 12:08 PM, Rob Herring wrote:
> > On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <[email protected]> wrote:
> >>
> >> In order to build drivers/irqchip/irq-bcm7120-l2.c as a module, we will
> >> need to have of_irq_count() exported to modules.
> >>
> >> Signed-off-by: Florian Fainelli <[email protected]>
> >> ---
> >> drivers/of/irq.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> >> index 352e14b007e7..949b9d1f8729 100644
> >> --- a/drivers/of/irq.c
> >> +++ b/drivers/of/irq.c
> >> @@ -440,6 +440,7 @@ int of_irq_count(struct device_node *dev)
> >>
> >> return nr;
> >> }
> >> +EXPORT_SYMBOL_GPL(of_irq_count);
> >
> > Please convert to use platform_irq_count() instead.
>
> That requires a platform_device to be passed to platform_irq_count(),
> will that work even when the drivers remain built into the kernel and
> get initialized early on?

No, does your irqchip using this do both? Looks to me like it is
always a platform_device.

Rob

2021-09-27 19:51:23

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 07/11] of/irq: Export of_irq_count to drivers

On 9/27/21 12:43 PM, Rob Herring wrote:
> On Mon, Sep 27, 2021 at 2:28 PM Florian Fainelli <[email protected]> wrote:
>>
>> On 9/27/21 12:08 PM, Rob Herring wrote:
>>> On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <[email protected]> wrote:
>>>>
>>>> In order to build drivers/irqchip/irq-bcm7120-l2.c as a module, we will
>>>> need to have of_irq_count() exported to modules.
>>>>
>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>> ---
>>>> drivers/of/irq.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>>>> index 352e14b007e7..949b9d1f8729 100644
>>>> --- a/drivers/of/irq.c
>>>> +++ b/drivers/of/irq.c
>>>> @@ -440,6 +440,7 @@ int of_irq_count(struct device_node *dev)
>>>>
>>>> return nr;
>>>> }
>>>> +EXPORT_SYMBOL_GPL(of_irq_count);
>>>
>>> Please convert to use platform_irq_count() instead.
>>
>> That requires a platform_device to be passed to platform_irq_count(),
>> will that work even when the drivers remain built into the kernel and
>> get initialized early on?
>
> No, does your irqchip using this do both? Looks to me like it is
> always a platform_device.

On ARM/ARM64 not using GKI as well as MIPS, we would want the module to
be built into the kernel image, however when using GKI that driver would
become a module. How do you suggest reconciling both usages?
--
Florian

2021-09-27 20:11:36

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 07/11] of/irq: Export of_irq_count to drivers

On Mon, Sep 27, 2021 at 2:49 PM Florian Fainelli <[email protected]> wrote:
>
> On 9/27/21 12:43 PM, Rob Herring wrote:
> > On Mon, Sep 27, 2021 at 2:28 PM Florian Fainelli <[email protected]> wrote:
> >>
> >> On 9/27/21 12:08 PM, Rob Herring wrote:
> >>> On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <[email protected]> wrote:
> >>>>
> >>>> In order to build drivers/irqchip/irq-bcm7120-l2.c as a module, we will
> >>>> need to have of_irq_count() exported to modules.
> >>>>
> >>>> Signed-off-by: Florian Fainelli <[email protected]>
> >>>> ---
> >>>> drivers/of/irq.c | 1 +
> >>>> 1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> >>>> index 352e14b007e7..949b9d1f8729 100644
> >>>> --- a/drivers/of/irq.c
> >>>> +++ b/drivers/of/irq.c
> >>>> @@ -440,6 +440,7 @@ int of_irq_count(struct device_node *dev)
> >>>>
> >>>> return nr;
> >>>> }
> >>>> +EXPORT_SYMBOL_GPL(of_irq_count);
> >>>
> >>> Please convert to use platform_irq_count() instead.
> >>
> >> That requires a platform_device to be passed to platform_irq_count(),
> >> will that work even when the drivers remain built into the kernel and
> >> get initialized early on?
> >
> > No, does your irqchip using this do both? Looks to me like it is
> > always a platform_device.
>
> On ARM/ARM64 not using GKI as well as MIPS, we would want the module to
> be built into the kernel image, however when using GKI that driver would
> become a module. How do you suggest reconciling both usages?

What's there to resolve? Every driver that works as a module can be
built-in. Is there something special about irqchip drivers?

The only issue I see here is platform_irqchip_probe() doesn't pass the
platform_device pointer to the irq_init_cb function. There's 3 ways to
fix that. Add a platform_device pointer to the init hook. That's a
global change though. That's the right thing to do IMO. Or you can use
of_find_device_by_node(). That's fairly expensive, but easy and
isolated. You could also set device_node.data pointer to the
platform_device, but ideally I'd like to get rid of that pointer as
it's hardly used.

Rob

2021-09-28 08:14:42

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH 07/11] of/irq: Export of_irq_count to drivers

On Mon, 27 Sep 2021 20:49:46 +0100,
Florian Fainelli <[email protected]> wrote:
>
> On 9/27/21 12:43 PM, Rob Herring wrote:
> > On Mon, Sep 27, 2021 at 2:28 PM Florian Fainelli <[email protected]> wrote:
> >>
> >> On 9/27/21 12:08 PM, Rob Herring wrote:
> >>> On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <[email protected]> wrote:
> >>>>
> >>>> In order to build drivers/irqchip/irq-bcm7120-l2.c as a module, we will
> >>>> need to have of_irq_count() exported to modules.
> >>>>
> >>>> Signed-off-by: Florian Fainelli <[email protected]>
> >>>> ---
> >>>> drivers/of/irq.c | 1 +
> >>>> 1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> >>>> index 352e14b007e7..949b9d1f8729 100644
> >>>> --- a/drivers/of/irq.c
> >>>> +++ b/drivers/of/irq.c
> >>>> @@ -440,6 +440,7 @@ int of_irq_count(struct device_node *dev)
> >>>>
> >>>> return nr;
> >>>> }
> >>>> +EXPORT_SYMBOL_GPL(of_irq_count);
> >>>
> >>> Please convert to use platform_irq_count() instead.
> >>
> >> That requires a platform_device to be passed to platform_irq_count(),
> >> will that work even when the drivers remain built into the kernel and
> >> get initialized early on?
> >
> > No, does your irqchip using this do both? Looks to me like it is
> > always a platform_device.
>
> On ARM/ARM64 not using GKI as well as MIPS, we would want the module to
> be built into the kernel image, however when using GKI that driver would
> become a module. How do you suggest reconciling both usages?

I don't see what GKI has to do with anything. Either the driver can be
built as a module (and it is in this case a platform device at all
times, built-in or not), or it cannot, and it falls into the
IRQCHIP_DECLARE() category (and there is no export problem).

Pick your poison!

Thanks,

M.

--
Without deviation from the norm, progress is not possible.