| Message ID | 20210924170546.805663-8-f.fainelli@gmail.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | Modular Broadcom irqchip drivers | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
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
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?
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
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?
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
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 --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
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(+)