diff mbox series

[07/11] of/irq: Export of_irq_count to drivers

Message ID 20210924170546.805663-8-f.fainelli@gmail.com
State Rejected
Headers show
Series Modular Broadcom irqchip drivers | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Florian Fainelli Sept. 24, 2021, 5:05 p.m. UTC
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 <f.fainelli@gmail.com>
---
 drivers/of/irq.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Rob Herring Sept. 27, 2021, 7:08 p.m. UTC | #1
On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com>
> ---
>  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
Florian Fainelli Sept. 27, 2021, 7:28 p.m. UTC | #2
On 9/27/21 12:08 PM, Rob Herring wrote:
> On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com>
>> ---
>>  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?
Rob Herring Sept. 27, 2021, 7:43 p.m. UTC | #3
On Mon, Sep 27, 2021 at 2:28 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 9/27/21 12:08 PM, Rob Herring wrote:
> > On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com>
> >> ---
> >>  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
Florian Fainelli Sept. 27, 2021, 7:49 p.m. UTC | #4
On 9/27/21 12:43 PM, Rob Herring wrote:
> On Mon, Sep 27, 2021 at 2:28 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> On 9/27/21 12:08 PM, Rob Herring wrote:
>>> On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com>
>>>> ---
>>>>  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?
Rob Herring Sept. 27, 2021, 8:09 p.m. UTC | #5
On Mon, Sep 27, 2021 at 2:49 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 9/27/21 12:43 PM, Rob Herring wrote:
> > On Mon, Sep 27, 2021 at 2:28 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>
> >> On 9/27/21 12:08 PM, Rob Herring wrote:
> >>> On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com>
> >>>> ---
> >>>>  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
Marc Zyngier Sept. 28, 2021, 8:10 a.m. UTC | #6
On Mon, 27 Sep 2021 20:49:46 +0100,
Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
> On 9/27/21 12:43 PM, Rob Herring wrote:
> > On Mon, Sep 27, 2021 at 2:28 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>
> >> On 9/27/21 12:08 PM, Rob Herring wrote:
> >>> On Fri, Sep 24, 2021 at 12:07 PM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com>
> >>>> ---
> >>>>  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.
diff mbox series

Patch

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