| Message ID | 20200616061338.109499-4-john.stultz@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Allow for qcom-pdc, pinctrl-msm and qcom-scm drivers to be loadable as modules | expand |
Hi, On 6/16/2020 11:43 AM, John Stultz wrote: > Allows qcom-pdc driver to be loaded as a permenent module typo: permanent > Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when > building as a module, we have to add the platform driver hooks > explicitly. > > Thanks to Saravana for his help on pointing out the > IRQCHIP_DECLARE issue and guidance on a solution. > > Cc: Andy Gross <agross@kernel.org> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Joerg Roedel <joro@8bytes.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Jason Cooper <jason@lakedaemon.net> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Lina Iyer <ilina@codeaurora.org> > Cc: Saravana Kannan <saravanak@google.com> > Cc: Todd Kjos <tkjos@google.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: linux-arm-msm@vger.kernel.org > Cc: iommu@lists.linux-foundation.org > Cc: linux-gpio@vger.kernel.org > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > drivers/irqchip/Kconfig | 2 +- > drivers/irqchip/qcom-pdc.c | 30 ++++++++++++++++++++++++++++++ > 2 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index 29fead208cad..12765bed08f9 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -425,7 +425,7 @@ config GOLDFISH_PIC > for Goldfish based virtual platforms. > > config QCOM_PDC > - bool "QCOM PDC" > + tristate "QCOM PDC" > depends on ARCH_QCOM > select IRQ_DOMAIN_HIERARCHY > help > diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c > index 6ae9e1f0819d..98d74160afcd 100644 > --- a/drivers/irqchip/qcom-pdc.c > +++ b/drivers/irqchip/qcom-pdc.c > @@ -11,7 +11,9 @@ > #include <linux/irqdomain.h> > #include <linux/io.h> > #include <linux/kernel.h> > +#include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_irq.h> please move this include in order after of_device.h > #include <linux/of_address.h> > #include <linux/of_device.h> > #include <linux/soc/qcom/irq.h> > @@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent) > return ret; > } > > +#ifdef MODULE > +static int qcom_pdc_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct device_node *parent = of_irq_find_parent(np); > + > + return qcom_pdc_init(np, parent); > +} > + > +static const struct of_device_id qcom_pdc_match_table[] = { > + { .compatible = "qcom,pdc" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, qcom_pdc_match_table); > + > +static struct platform_driver qcom_pdc_driver = { > + .probe = qcom_pdc_probe, > + .driver = { > + .name = "qcom-pdc", > + .of_match_table = qcom_pdc_match_table, can you please set .suppress_bind_attrs = true, This is to prevent bind/unbind using sysfs. Once irqchip driver module is loaded, it shouldn't get unbind at runtime. Thanks, Maulik > + }, > +}; > +module_platform_driver(qcom_pdc_driver); > +#else > IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init); > +#endif > + > +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller"); > +MODULE_LICENSE("GPL v2");
On Tue, Jun 16 2020 at 05:30 -0600, Maulik Shah wrote: >Hi, > >On 6/16/2020 11:43 AM, John Stultz wrote: >>Allows qcom-pdc driver to be loaded as a permenent module > >typo: permanent > >>Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when >>building as a module, we have to add the platform driver hooks >>explicitly. >> >>Thanks to Saravana for his help on pointing out the >>IRQCHIP_DECLARE issue and guidance on a solution. >> >>Cc: Andy Gross <agross@kernel.org> >>Cc: Bjorn Andersson <bjorn.andersson@linaro.org> >>Cc: Joerg Roedel <joro@8bytes.org> >>Cc: Thomas Gleixner <tglx@linutronix.de> >>Cc: Jason Cooper <jason@lakedaemon.net> >>Cc: Marc Zyngier <maz@kernel.org> >>Cc: Linus Walleij <linus.walleij@linaro.org> >>Cc: Lina Iyer <ilina@codeaurora.org> >>Cc: Saravana Kannan <saravanak@google.com> >>Cc: Todd Kjos <tkjos@google.com> >>Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>Cc: linux-arm-msm@vger.kernel.org >>Cc: iommu@lists.linux-foundation.org >>Cc: linux-gpio@vger.kernel.org >>Signed-off-by: John Stultz <john.stultz@linaro.org> >>--- >> drivers/irqchip/Kconfig | 2 +- >> drivers/irqchip/qcom-pdc.c | 30 ++++++++++++++++++++++++++++++ >> 2 files changed, 31 insertions(+), 1 deletion(-) >> >>diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig >>index 29fead208cad..12765bed08f9 100644 >>--- a/drivers/irqchip/Kconfig >>+++ b/drivers/irqchip/Kconfig >>@@ -425,7 +425,7 @@ config GOLDFISH_PIC >> for Goldfish based virtual platforms. >> config QCOM_PDC >>- bool "QCOM PDC" >>+ tristate "QCOM PDC" >> depends on ARCH_QCOM >> select IRQ_DOMAIN_HIERARCHY >> help >>diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c >>index 6ae9e1f0819d..98d74160afcd 100644 >>--- a/drivers/irqchip/qcom-pdc.c >>+++ b/drivers/irqchip/qcom-pdc.c >>@@ -11,7 +11,9 @@ >> #include <linux/irqdomain.h> >> #include <linux/io.h> >> #include <linux/kernel.h> >>+#include <linux/module.h> >> #include <linux/of.h> >>+#include <linux/of_irq.h> >please move this include in order after of_device.h >> #include <linux/of_address.h> >> #include <linux/of_device.h> >> #include <linux/soc/qcom/irq.h> >>@@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent) >> return ret; >> } >>+#ifdef MODULE >>+static int qcom_pdc_probe(struct platform_device *pdev) >>+{ >>+ struct device_node *np = pdev->dev.of_node; >>+ struct device_node *parent = of_irq_find_parent(np); >>+ >>+ return qcom_pdc_init(np, parent); >>+} >>+ >>+static const struct of_device_id qcom_pdc_match_table[] = { >>+ { .compatible = "qcom,pdc" }, >>+ {} >>+}; >>+MODULE_DEVICE_TABLE(of, qcom_pdc_match_table); >>+ >>+static struct platform_driver qcom_pdc_driver = { >>+ .probe = qcom_pdc_probe, >>+ .driver = { >>+ .name = "qcom-pdc", >>+ .of_match_table = qcom_pdc_match_table, > >can you please set .suppress_bind_attrs = true, > >This is to prevent bind/unbind using sysfs. Once irqchip driver module >is loaded, it shouldn't get unbind at runtime. > That is a good point. We probably should do that to RPMH RSC driver as well. >Thanks, >Maulik >>+ }, >>+}; >>+module_platform_driver(qcom_pdc_driver); >>+#else >> IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init); >>+#endif >>+ >>+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller"); >>+MODULE_LICENSE("GPL v2"); > >-- >QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation >
On Tue, Jun 16, 2020 at 4:30 AM Maulik Shah <mkshah@codeaurora.org> wrote: > > Hi, > > On 6/16/2020 11:43 AM, John Stultz wrote: > > Allows qcom-pdc driver to be loaded as a permenent module > > typo: permanent > > > Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when > > building as a module, we have to add the platform driver hooks > > explicitly. > > > > Thanks to Saravana for his help on pointing out the > > IRQCHIP_DECLARE issue and guidance on a solution. > > > > Cc: Andy Gross <agross@kernel.org> > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: Joerg Roedel <joro@8bytes.org> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Jason Cooper <jason@lakedaemon.net> > > Cc: Marc Zyngier <maz@kernel.org> > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Cc: Lina Iyer <ilina@codeaurora.org> > > Cc: Saravana Kannan <saravanak@google.com> > > Cc: Todd Kjos <tkjos@google.com> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: linux-arm-msm@vger.kernel.org > > Cc: iommu@lists.linux-foundation.org > > Cc: linux-gpio@vger.kernel.org > > Signed-off-by: John Stultz <john.stultz@linaro.org> > > --- > > drivers/irqchip/Kconfig | 2 +- > > drivers/irqchip/qcom-pdc.c | 30 ++++++++++++++++++++++++++++++ > > 2 files changed, 31 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > > index 29fead208cad..12765bed08f9 100644 > > --- a/drivers/irqchip/Kconfig > > +++ b/drivers/irqchip/Kconfig > > @@ -425,7 +425,7 @@ config GOLDFISH_PIC > > for Goldfish based virtual platforms. > > > > config QCOM_PDC > > - bool "QCOM PDC" > > + tristate "QCOM PDC" > > depends on ARCH_QCOM > > select IRQ_DOMAIN_HIERARCHY > > help > > diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c > > index 6ae9e1f0819d..98d74160afcd 100644 > > --- a/drivers/irqchip/qcom-pdc.c > > +++ b/drivers/irqchip/qcom-pdc.c > > @@ -11,7 +11,9 @@ > > #include <linux/irqdomain.h> > > #include <linux/io.h> > > #include <linux/kernel.h> > > +#include <linux/module.h> > > #include <linux/of.h> > > +#include <linux/of_irq.h> > please move this include in order after of_device.h > > #include <linux/of_address.h> > > #include <linux/of_device.h> > > #include <linux/soc/qcom/irq.h> > > @@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent) > > return ret; > > } > > > > +#ifdef MODULE > > +static int qcom_pdc_probe(struct platform_device *pdev) > > +{ > > + struct device_node *np = pdev->dev.of_node; > > + struct device_node *parent = of_irq_find_parent(np); > > + > > + return qcom_pdc_init(np, parent); > > +} > > + > > +static const struct of_device_id qcom_pdc_match_table[] = { > > + { .compatible = "qcom,pdc" }, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(of, qcom_pdc_match_table); > > + > > +static struct platform_driver qcom_pdc_driver = { > > + .probe = qcom_pdc_probe, > > + .driver = { > > + .name = "qcom-pdc", > > + .of_match_table = qcom_pdc_match_table, > > can you please set .suppress_bind_attrs = true, > > This is to prevent bind/unbind using sysfs. Once irqchip driver module > is loaded, it shouldn't get unbind at runtime. Thanks, I really appreciate the review! I've made these changes on my side and they'll be included in v2. thanks -john
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 29fead208cad..12765bed08f9 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -425,7 +425,7 @@ config GOLDFISH_PIC for Goldfish based virtual platforms. config QCOM_PDC - bool "QCOM PDC" + tristate "QCOM PDC" depends on ARCH_QCOM select IRQ_DOMAIN_HIERARCHY help diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c index 6ae9e1f0819d..98d74160afcd 100644 --- a/drivers/irqchip/qcom-pdc.c +++ b/drivers/irqchip/qcom-pdc.c @@ -11,7 +11,9 @@ #include <linux/irqdomain.h> #include <linux/io.h> #include <linux/kernel.h> +#include <linux/module.h> #include <linux/of.h> +#include <linux/of_irq.h> #include <linux/of_address.h> #include <linux/of_device.h> #include <linux/soc/qcom/irq.h> @@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent) return ret; } +#ifdef MODULE +static int qcom_pdc_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct device_node *parent = of_irq_find_parent(np); + + return qcom_pdc_init(np, parent); +} + +static const struct of_device_id qcom_pdc_match_table[] = { + { .compatible = "qcom,pdc" }, + {} +}; +MODULE_DEVICE_TABLE(of, qcom_pdc_match_table); + +static struct platform_driver qcom_pdc_driver = { + .probe = qcom_pdc_probe, + .driver = { + .name = "qcom-pdc", + .of_match_table = qcom_pdc_match_table, + }, +}; +module_platform_driver(qcom_pdc_driver); +#else IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init); +#endif + +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller"); +MODULE_LICENSE("GPL v2");
Allows qcom-pdc driver to be loaded as a permenent module Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when building as a module, we have to add the platform driver hooks explicitly. Thanks to Saravana for his help on pointing out the IRQCHIP_DECLARE issue and guidance on a solution. Cc: Andy Gross <agross@kernel.org> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Joerg Roedel <joro@8bytes.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Jason Cooper <jason@lakedaemon.net> Cc: Marc Zyngier <maz@kernel.org> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Lina Iyer <ilina@codeaurora.org> Cc: Saravana Kannan <saravanak@google.com> Cc: Todd Kjos <tkjos@google.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: linux-arm-msm@vger.kernel.org Cc: iommu@lists.linux-foundation.org Cc: linux-gpio@vger.kernel.org Signed-off-by: John Stultz <john.stultz@linaro.org> --- drivers/irqchip/Kconfig | 2 +- drivers/irqchip/qcom-pdc.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)