| Message ID | 1517289851-14111-3-git-send-email-mmaddireddy@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add loadable kernel module and power management support | expand |
On Tue, Jan 30, 2018 at 10:54:09AM +0530, Manikanta Maddireddy wrote: > Implement remove callback function for Tegra PCIe driver to add > loadable kernel module support. Change PCI_TEGRA config to tristate to > allow pci-tegra driver to be build as a module. > > Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> > Acked-by: Thierry Reding <treding@nvidia.com> > --- > V2: > * no change in this patch > V3: > * use tegra_pcie_debugfs_exit() helper function in tegra_pcie_debugfs_init() > V4: > * no change in this patch > V5: > * Decoupled from https://patchwork.ozlabs.org/patch/832053/ and > rebased on linux-next > V6: > * no change in this patch > V7: > * no change in this patch > > drivers/pci/host/Kconfig | 2 +- > drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++--- > 2 files changed, 32 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index 38d12980db0f..6fd2a5937804 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -34,7 +34,7 @@ config PCI_FTPCI100 > default ARCH_GEMINI > > config PCI_TEGRA > - bool "NVIDIA Tegra PCIe controller" > + tristate "NVIDIA Tegra PCIe controller" > depends on ARCH_TEGRA > help > Say Y here if you want support for the PCIe host controller found > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c > index e68507f658d8..981f126b14d6 100644 > --- a/drivers/pci/host/pci-tegra.c > +++ b/drivers/pci/host/pci-tegra.c > @@ -35,6 +35,7 @@ > #include <linux/irqdomain.h> > #include <linux/kernel.h> > #include <linux/init.h> > +#include <linux/module.h> > #include <linux/msi.h> > #include <linux/of_address.h> > #include <linux/of_pci.h> > @@ -2276,6 +2277,12 @@ static const struct file_operations tegra_pcie_ports_ops = { > .release = seq_release, > }; > > +static void tegra_pcie_debugfs_exit(struct tegra_pcie *pcie) > +{ > + debugfs_remove_recursive(pcie->debugfs); > + pcie->debugfs = NULL; > +} > + > static int tegra_pcie_debugfs_init(struct tegra_pcie *pcie) > { > struct dentry *file; > @@ -2292,8 +2299,7 @@ static int tegra_pcie_debugfs_init(struct tegra_pcie *pcie) > return 0; > > remove: > - debugfs_remove_recursive(pcie->debugfs); > - pcie->debugfs = NULL; > + tegra_pcie_debugfs_exit(pcie); > return -ENOMEM; > } > > @@ -2311,6 +2317,7 @@ static int tegra_pcie_probe(struct platform_device *pdev) > > pcie = pci_host_bridge_priv(host); > host->sysdata = pcie; > + platform_set_drvdata(pdev, pcie); > > pcie->soc = of_device_get_match_data(dev); > INIT_LIST_HEAD(&pcie->ports); > @@ -2388,6 +2395,25 @@ static int tegra_pcie_probe(struct platform_device *pdev) > return err; > } > > +static int tegra_pcie_remove(struct platform_device *pdev) > +{ > + struct tegra_pcie *pcie = platform_get_drvdata(pdev); > + struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie); > + > + if (IS_ENABLED(CONFIG_DEBUG_FS)) > + tegra_pcie_debugfs_exit(pcie); > + pci_stop_root_bus(host->bus); > + pci_remove_root_bus(host->bus); > + tegra_pcie_disable_ports(pcie); > + if (IS_ENABLED(CONFIG_PCI_MSI)) > + tegra_pcie_disable_msi(pcie); > + tegra_pcie_free_resources(pcie); > + tegra_pcie_disable_controller(pcie); > + tegra_pcie_put_resources(pcie); I prefer blank lines around conditional blocks for readability, so the above would be: if (IS_ENABLED(CONFIG_DEBUG_FS)) tegra_pcie_debugfs_exit(pcie); pci_stop_root_bus(host->bus); pci_remove_root_bus(host->bus); tegra_pcie_disable_ports(pcie); if (IS_ENABLED(CONFIG_PCI_MSI)) tegra_pcie_disable_msi(pcie); tegra_pcie_free_resources(pcie); tegra_pcie_disable_controller(pcie); tegra_pcie_put_resources(pcie); Not sure if Lorenzo or Bjorn have any specific preferences, though. My Acked-by remains valid, though, since this is nitpicky.
On Tue, Jan 30, 2018 at 10:54:09AM +0530, Manikanta Maddireddy wrote: > Implement remove callback function for Tegra PCIe driver to add > loadable kernel module support. Change PCI_TEGRA config to tristate to > allow pci-tegra driver to be build as a module. > > Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> > Acked-by: Thierry Reding <treding@nvidia.com> > --- > V2: > * no change in this patch > V3: > * use tegra_pcie_debugfs_exit() helper function in tegra_pcie_debugfs_init() > V4: > * no change in this patch > V5: > * Decoupled from https://patchwork.ozlabs.org/patch/832053/ and > rebased on linux-next > V6: > * no change in this patch > V7: > * no change in this patch > > drivers/pci/host/Kconfig | 2 +- > drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++--- > 2 files changed, 32 insertions(+), 4 deletions(-) Trying to build this as a module I get a link failure because irq_set_msi_desc() is not exported. I thought a patch to do that had been part of an earlier version of this series. Has that not been merged yet? Thierry
On 13-Feb-18 6:37 PM, Thierry Reding wrote: > On Tue, Jan 30, 2018 at 10:54:09AM +0530, Manikanta Maddireddy wrote: >> Implement remove callback function for Tegra PCIe driver to add >> loadable kernel module support. Change PCI_TEGRA config to tristate to >> allow pci-tegra driver to be build as a module. >> >> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> >> Acked-by: Thierry Reding <treding@nvidia.com> >> --- >> V2: >> * no change in this patch >> V3: >> * use tegra_pcie_debugfs_exit() helper function in tegra_pcie_debugfs_init() >> V4: >> * no change in this patch >> V5: >> * Decoupled from https://patchwork.ozlabs.org/patch/832053/ and >> rebased on linux-next >> V6: >> * no change in this patch >> V7: >> * no change in this patch >> >> drivers/pci/host/Kconfig | 2 +- >> drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++--- >> 2 files changed, 32 insertions(+), 4 deletions(-) > > Trying to build this as a module I get a link failure because > irq_set_msi_desc() is not exported. I thought a patch to do that had > been part of an earlier version of this series. Has that not been merged > yet? > > Thierry > I got a review comment to use generic MSI model instead of deprecated API. source: https://patchwork.ozlabs.org/patch/841111/ Also arm32 compilation will fail because of missing EXPORT of tegra_cpuidle_pcie_irqs_in_use() source: https://patchwork.ozlabs.org/patch/841119/ I need to put some time to prepare patches for these two cases. Meanwhile I want to get the basic driver structure for LKM to be merged. I verified locally with these two symbols exported.
On Tue, Feb 13, 2018 at 06:48:49PM +0530, Manikanta Maddireddy wrote: > > > On 13-Feb-18 6:37 PM, Thierry Reding wrote: > > On Tue, Jan 30, 2018 at 10:54:09AM +0530, Manikanta Maddireddy wrote: > >> Implement remove callback function for Tegra PCIe driver to add > >> loadable kernel module support. Change PCI_TEGRA config to tristate to > >> allow pci-tegra driver to be build as a module. > >> > >> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> > >> Acked-by: Thierry Reding <treding@nvidia.com> > >> --- > >> V2: > >> * no change in this patch > >> V3: > >> * use tegra_pcie_debugfs_exit() helper function in tegra_pcie_debugfs_init() > >> V4: > >> * no change in this patch > >> V5: > >> * Decoupled from https://patchwork.ozlabs.org/patch/832053/ and > >> rebased on linux-next > >> V6: > >> * no change in this patch > >> V7: > >> * no change in this patch > >> > >> drivers/pci/host/Kconfig | 2 +- > >> drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++--- > >> 2 files changed, 32 insertions(+), 4 deletions(-) > > > > Trying to build this as a module I get a link failure because > > irq_set_msi_desc() is not exported. I thought a patch to do that had > > been part of an earlier version of this series. Has that not been merged > > yet? > > > > Thierry > > > I got a review comment to use generic MSI model instead of deprecated API. > source: https://patchwork.ozlabs.org/patch/841111/ > > Also arm32 compilation will fail because of missing EXPORT of > tegra_cpuidle_pcie_irqs_in_use() > source: https://patchwork.ozlabs.org/patch/841119/ > > I need to put some time to prepare patches for these two cases. > Meanwhile I want to get the basic driver structure for LKM to be merged. > I verified locally with these two symbols exported. Okay, that's fine. However, you can't request these patches to be merged if they will break builds. There are people that run automated builders that will check random configurations, which is bound to eventually get us a bug report about how this fails to build. Perhaps in order to get loadable module support merged without these issues you could omit the Kconfig change making this a tristate symbol. That way we can get all the structural changes ready and you can work on the necessary exports or MSI model patches so that we can eventually flip the symbol to tristate. By the way, if you do respin for the tristate change, do you mind taking a look at the blank line changes for readability I mentioned earlier? Thierry
On 13-Feb-18 7:21 PM, Thierry Reding wrote: > On Tue, Feb 13, 2018 at 06:48:49PM +0530, Manikanta Maddireddy wrote: >> >> >> On 13-Feb-18 6:37 PM, Thierry Reding wrote: >>> On Tue, Jan 30, 2018 at 10:54:09AM +0530, Manikanta Maddireddy wrote: >>>> Implement remove callback function for Tegra PCIe driver to add >>>> loadable kernel module support. Change PCI_TEGRA config to tristate to >>>> allow pci-tegra driver to be build as a module. >>>> >>>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> >>>> Acked-by: Thierry Reding <treding@nvidia.com> >>>> --- >>>> V2: >>>> * no change in this patch >>>> V3: >>>> * use tegra_pcie_debugfs_exit() helper function in tegra_pcie_debugfs_init() >>>> V4: >>>> * no change in this patch >>>> V5: >>>> * Decoupled from https://patchwork.ozlabs.org/patch/832053/ and >>>> rebased on linux-next >>>> V6: >>>> * no change in this patch >>>> V7: >>>> * no change in this patch >>>> >>>> drivers/pci/host/Kconfig | 2 +- >>>> drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++--- >>>> 2 files changed, 32 insertions(+), 4 deletions(-) >>> >>> Trying to build this as a module I get a link failure because >>> irq_set_msi_desc() is not exported. I thought a patch to do that had >>> been part of an earlier version of this series. Has that not been merged >>> yet? >>> >>> Thierry >>> >> I got a review comment to use generic MSI model instead of deprecated API. >> source: https://patchwork.ozlabs.org/patch/841111/ >> >> Also arm32 compilation will fail because of missing EXPORT of >> tegra_cpuidle_pcie_irqs_in_use() >> source: https://patchwork.ozlabs.org/patch/841119/ >> >> I need to put some time to prepare patches for these two cases. >> Meanwhile I want to get the basic driver structure for LKM to be merged. >> I verified locally with these two symbols exported. > > Okay, that's fine. However, you can't request these patches to be merged > if they will break builds. There are people that run automated builders > that will check random configurations, which is bound to eventually get > us a bug report about how this fails to build. > > Perhaps in order to get loadable module support merged without these > issues you could omit the Kconfig change making this a tristate symbol. > That way we can get all the structural changes ready and you can work > on the necessary exports or MSI model patches so that we can eventually > flip the symbol to tristate. > > By the way, if you do respin for the tristate change, do you mind taking > a look at the blank line changes for readability I mentioned earlier? > > Thierry > Taken care of both and published V8. Manikanta
On 13-Feb-18 7:21 PM, Thierry Reding wrote: > On Tue, Feb 13, 2018 at 06:48:49PM +0530, Manikanta Maddireddy wrote: >> >> >> On 13-Feb-18 6:37 PM, Thierry Reding wrote: >>> On Tue, Jan 30, 2018 at 10:54:09AM +0530, Manikanta Maddireddy wrote: >>>> Implement remove callback function for Tegra PCIe driver to add >>>> loadable kernel module support. Change PCI_TEGRA config to tristate to >>>> allow pci-tegra driver to be build as a module. >>>> >>>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> >>>> Acked-by: Thierry Reding <treding@nvidia.com> >>>> --- >>>> V2: >>>> * no change in this patch >>>> V3: >>>> * use tegra_pcie_debugfs_exit() helper function in tegra_pcie_debugfs_init() >>>> V4: >>>> * no change in this patch >>>> V5: >>>> * Decoupled from https://patchwork.ozlabs.org/patch/832053/ and >>>> rebased on linux-next >>>> V6: >>>> * no change in this patch >>>> V7: >>>> * no change in this patch >>>> >>>> drivers/pci/host/Kconfig | 2 +- >>>> drivers/pci/host/pci-tegra.c | 34 +++++++++++++++++++++++++++++++--- >>>> 2 files changed, 32 insertions(+), 4 deletions(-) >>> >>> Trying to build this as a module I get a link failure because >>> irq_set_msi_desc() is not exported. I thought a patch to do that had >>> been part of an earlier version of this series. Has that not been merged >>> yet? >>> >>> Thierry >>> >> I got a review comment to use generic MSI model instead of deprecated API. >> source: https://patchwork.ozlabs.org/patch/841111/ >> >> Also arm32 compilation will fail because of missing EXPORT of >> tegra_cpuidle_pcie_irqs_in_use() >> source: https://patchwork.ozlabs.org/patch/841119/ >> >> I need to put some time to prepare patches for these two cases. >> Meanwhile I want to get the basic driver structure for LKM to be merged. >> I verified locally with these two symbols exported. > > Okay, that's fine. However, you can't request these patches to be merged > if they will break builds. There are people that run automated builders > that will check random configurations, which is bound to eventually get > us a bug report about how this fails to build. > > Perhaps in order to get loadable module support merged without these > issues you could omit the Kconfig change making this a tristate symbol. > That way we can get all the structural changes ready and you can work > on the necessary exports or MSI model patches so that we can eventually > flip the symbol to tristate. > > By the way, if you do respin for the tristate change, do you mind taking > a look at the blank line changes for readability I mentioned earlier? > > Thierry > Hi Thierry, Taken care of both and published V8. Since these are simple changes, I retained your Ack & Tested signoff. Manikanta
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index 38d12980db0f..6fd2a5937804 100644 --- a/drivers/pci/host/Kconfig +++ b/drivers/pci/host/Kconfig @@ -34,7 +34,7 @@ config PCI_FTPCI100 default ARCH_GEMINI config PCI_TEGRA - bool "NVIDIA Tegra PCIe controller" + tristate "NVIDIA Tegra PCIe controller" depends on ARCH_TEGRA help Say Y here if you want support for the PCIe host controller found diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c index e68507f658d8..981f126b14d6 100644 --- a/drivers/pci/host/pci-tegra.c +++ b/drivers/pci/host/pci-tegra.c @@ -35,6 +35,7 @@ #include <linux/irqdomain.h> #include <linux/kernel.h> #include <linux/init.h> +#include <linux/module.h> #include <linux/msi.h> #include <linux/of_address.h> #include <linux/of_pci.h> @@ -2276,6 +2277,12 @@ static const struct file_operations tegra_pcie_ports_ops = { .release = seq_release, }; +static void tegra_pcie_debugfs_exit(struct tegra_pcie *pcie) +{ + debugfs_remove_recursive(pcie->debugfs); + pcie->debugfs = NULL; +} + static int tegra_pcie_debugfs_init(struct tegra_pcie *pcie) { struct dentry *file; @@ -2292,8 +2299,7 @@ static int tegra_pcie_debugfs_init(struct tegra_pcie *pcie) return 0; remove: - debugfs_remove_recursive(pcie->debugfs); - pcie->debugfs = NULL; + tegra_pcie_debugfs_exit(pcie); return -ENOMEM; } @@ -2311,6 +2317,7 @@ static int tegra_pcie_probe(struct platform_device *pdev) pcie = pci_host_bridge_priv(host); host->sysdata = pcie; + platform_set_drvdata(pdev, pcie); pcie->soc = of_device_get_match_data(dev); INIT_LIST_HEAD(&pcie->ports); @@ -2388,6 +2395,25 @@ static int tegra_pcie_probe(struct platform_device *pdev) return err; } +static int tegra_pcie_remove(struct platform_device *pdev) +{ + struct tegra_pcie *pcie = platform_get_drvdata(pdev); + struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie); + + if (IS_ENABLED(CONFIG_DEBUG_FS)) + tegra_pcie_debugfs_exit(pcie); + pci_stop_root_bus(host->bus); + pci_remove_root_bus(host->bus); + tegra_pcie_disable_ports(pcie); + if (IS_ENABLED(CONFIG_PCI_MSI)) + tegra_pcie_disable_msi(pcie); + tegra_pcie_free_resources(pcie); + tegra_pcie_disable_controller(pcie); + tegra_pcie_put_resources(pcie); + + return 0; +} + static struct platform_driver tegra_pcie_driver = { .driver = { .name = "tegra-pcie", @@ -2395,5 +2421,7 @@ static struct platform_driver tegra_pcie_driver = { .suppress_bind_attrs = true, }, .probe = tegra_pcie_probe, + .remove = tegra_pcie_remove, }; -builtin_platform_driver(tegra_pcie_driver); +module_platform_driver(tegra_pcie_driver); +MODULE_LICENSE("GPL");