| Message ID | 1556081835-12921-1-git-send-email-ley.foon.tan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: altera: Allow building as module | expand |
On Wed, Apr 24, 2019 at 12:57 PM Ley Foon Tan <ley.foon.tan@intel.com> wrote: > > Altera PCIe Rootport IP is a soft IP and is only available after > FPGA image is programmed. > > Make driver modulable to support use case FPGA image is programmed > after kernel is booted. User proram FPGA image in kernel then only load > PCIe driver module. > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > --- > drivers/pci/controller/Kconfig | 2 +- > drivers/pci/controller/pcie-altera.c | 28 ++++++++++++++++++++++++++-- > 2 files changed, 27 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > index 6012f3059acd..4b550f9cdd56 100644 > --- a/drivers/pci/controller/Kconfig > +++ b/drivers/pci/controller/Kconfig > @@ -174,7 +174,7 @@ config PCIE_IPROC_MSI > PCIe controller > > config PCIE_ALTERA > - bool "Altera PCIe controller" > + tristate "Altera PCIe controller" > depends on ARM || NIOS2 || ARM64 || COMPILE_TEST > help > Say Y here if you want to enable PCIe controller support on Altera > diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c > index 27edcebd1726..6c86bc69ace8 100644 > --- a/drivers/pci/controller/pcie-altera.c > +++ b/drivers/pci/controller/pcie-altera.c > @@ -10,6 +10,7 @@ > #include <linux/interrupt.h> > #include <linux/irqchip/chained_irq.h> > #include <linux/init.h> > +#include <linux/module.h> > #include <linux/of_address.h> > #include <linux/of_device.h> > #include <linux/of_irq.h> > @@ -705,6 +706,13 @@ static int altera_pcie_init_irq_domain(struct altera_pcie *pcie) > return 0; > } > > +static int altera_pcie_irq_teardown(struct altera_pcie *pcie) > +{ > + irq_set_chained_handler_and_data(pcie->irq, NULL, NULL); > + irq_domain_remove(pcie->irq_domain); > + irq_dispose_mapping(pcie->irq); > +} > + > static int altera_pcie_parse_dt(struct altera_pcie *pcie) > { > struct device *dev = &pcie->pdev->dev; > @@ -798,6 +806,7 @@ static int altera_pcie_probe(struct platform_device *pdev) > > pcie = pci_host_bridge_priv(bridge); > pcie->pdev = pdev; > + platform_set_drvdata(pdev, pcie); > > match = of_match_device(altera_pcie_of_match, &pdev->dev); > if (!match) > @@ -855,13 +864,28 @@ static int altera_pcie_probe(struct platform_device *pdev) > return ret; > } > > +static int altera_pcie_remove(struct platform_device *pdev) > +{ > + struct altera_pcie *pcie = platform_get_drvdata(pdev); > + struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie); > + > + pci_stop_root_bus(bridge->bus); > + pci_remove_root_bus(bridge->bus); > + pci_free_resource_list(&pcie->resources); > + altera_pcie_irq_teardown(pcie); > + > + return 0; > +} > + > static struct platform_driver altera_pcie_driver = { > .probe = altera_pcie_probe, > + .remove = altera_pcie_remove, > .driver = { > .name = "altera-pcie", > .of_match_table = altera_pcie_of_match, > - .suppress_bind_attrs = true, > }, > }; > > -builtin_platform_driver(altera_pcie_driver); > +MODULE_DEVICE_TABLE(of, altera_pcie_of_match); > +module_platform_driver(altera_pcie_driver); > +MODULE_LICENSE("GPL v2"); > -- > 2.19.0 > Hi Any comment for this patch? Regards Ley Foon
On Tue, May 14, 2019 at 01:35:05PM +0800, Ley Foon Tan wrote: > On Wed, Apr 24, 2019 at 12:57 PM Ley Foon Tan <ley.foon.tan@intel.com> wrote: > > > > Altera PCIe Rootport IP is a soft IP and is only available after > > FPGA image is programmed. > > > > Make driver modulable to support use case FPGA image is programmed > > after kernel is booted. User proram FPGA image in kernel then only load > > PCIe driver module. > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > > --- > > drivers/pci/controller/Kconfig | 2 +- > > drivers/pci/controller/pcie-altera.c | 28 ++++++++++++++++++++++++++-- > > 2 files changed, 27 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > > index 6012f3059acd..4b550f9cdd56 100644 > > --- a/drivers/pci/controller/Kconfig > > +++ b/drivers/pci/controller/Kconfig > > @@ -174,7 +174,7 @@ config PCIE_IPROC_MSI > > PCIe controller > > > > config PCIE_ALTERA > > - bool "Altera PCIe controller" > > + tristate "Altera PCIe controller" > > depends on ARM || NIOS2 || ARM64 || COMPILE_TEST > > help > > Say Y here if you want to enable PCIe controller support on Altera > > diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c > > index 27edcebd1726..6c86bc69ace8 100644 > > --- a/drivers/pci/controller/pcie-altera.c > > +++ b/drivers/pci/controller/pcie-altera.c > > @@ -10,6 +10,7 @@ > > #include <linux/interrupt.h> > > #include <linux/irqchip/chained_irq.h> > > #include <linux/init.h> > > +#include <linux/module.h> > > #include <linux/of_address.h> > > #include <linux/of_device.h> > > #include <linux/of_irq.h> > > @@ -705,6 +706,13 @@ static int altera_pcie_init_irq_domain(struct altera_pcie *pcie) > > return 0; > > } > > > > +static int altera_pcie_irq_teardown(struct altera_pcie *pcie) > > +{ > > + irq_set_chained_handler_and_data(pcie->irq, NULL, NULL); > > + irq_domain_remove(pcie->irq_domain); > > + irq_dispose_mapping(pcie->irq); > > +} > > + > > static int altera_pcie_parse_dt(struct altera_pcie *pcie) > > { > > struct device *dev = &pcie->pdev->dev; > > @@ -798,6 +806,7 @@ static int altera_pcie_probe(struct platform_device *pdev) > > > > pcie = pci_host_bridge_priv(bridge); > > pcie->pdev = pdev; > > + platform_set_drvdata(pdev, pcie); > > > > match = of_match_device(altera_pcie_of_match, &pdev->dev); > > if (!match) > > @@ -855,13 +864,28 @@ static int altera_pcie_probe(struct platform_device *pdev) > > return ret; > > } > > > > +static int altera_pcie_remove(struct platform_device *pdev) > > +{ > > + struct altera_pcie *pcie = platform_get_drvdata(pdev); > > + struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie); > > + > > + pci_stop_root_bus(bridge->bus); > > + pci_remove_root_bus(bridge->bus); > > + pci_free_resource_list(&pcie->resources); > > + altera_pcie_irq_teardown(pcie); > > + > > + return 0; > > +} > > + > > static struct platform_driver altera_pcie_driver = { > > .probe = altera_pcie_probe, > > + .remove = altera_pcie_remove, > > .driver = { > > .name = "altera-pcie", > > .of_match_table = altera_pcie_of_match, > > - .suppress_bind_attrs = true, > > }, > > }; > > > > -builtin_platform_driver(altera_pcie_driver); > > +MODULE_DEVICE_TABLE(of, altera_pcie_of_match); > > +module_platform_driver(altera_pcie_driver); > > +MODULE_LICENSE("GPL v2"); > > -- > > 2.19.0 > > > Hi > > Any comment for this patch? Applied to pci/altera for v5.3, thanks. Lorenzo
On Wed, Apr 24, 2019 at 12:57:14PM +0800, Ley Foon Tan wrote: > Altera PCIe Rootport IP is a soft IP and is only available after > FPGA image is programmed. > > Make driver modulable to support use case FPGA image is programmed > after kernel is booted. User proram FPGA image in kernel then only load > PCIe driver module. I'm not objecting to these patches, but help me understand how this works. The "usual" scenario is that if a driver is loaded before a matching device is available, i.e., either the driver is built statically or it is loaded before a device is hot-added, the event of the device being available causes the driver's probe method to be called. This seems to be a more manual process of programming the FPGA which results in a new "altera-pcie" platform device. And then apparently you need to load the appropriate module by hand? Is there no "hot-add" type of event for this platform device that automatically looks for the driver? Bjorn
On Tue, Jun 4, 2019 at 9:18 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Wed, Apr 24, 2019 at 12:57:14PM +0800, Ley Foon Tan wrote: > > Altera PCIe Rootport IP is a soft IP and is only available after > > FPGA image is programmed. > > > > Make driver modulable to support use case FPGA image is programmed > > after kernel is booted. User proram FPGA image in kernel then only load > > PCIe driver module. > > I'm not objecting to these patches, but help me understand how this > works. The "usual" scenario is that if a driver is loaded before a > matching device is available, i.e., either the driver is built > statically or it is loaded before a device is hot-added, the event of > the device being available causes the driver's probe method to be > called. > > This seems to be a more manual process of programming the FPGA which > results in a new "altera-pcie" platform device. And then apparently > you need to load the appropriate module by hand? Is there no > "hot-add" type of event for this platform device that automatically > looks for the driver? Yes, we need load module manually now. Regards Ley Foon
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig index 6012f3059acd..4b550f9cdd56 100644 --- a/drivers/pci/controller/Kconfig +++ b/drivers/pci/controller/Kconfig @@ -174,7 +174,7 @@ config PCIE_IPROC_MSI PCIe controller config PCIE_ALTERA - bool "Altera PCIe controller" + tristate "Altera PCIe controller" depends on ARM || NIOS2 || ARM64 || COMPILE_TEST help Say Y here if you want to enable PCIe controller support on Altera diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c index 27edcebd1726..6c86bc69ace8 100644 --- a/drivers/pci/controller/pcie-altera.c +++ b/drivers/pci/controller/pcie-altera.c @@ -10,6 +10,7 @@ #include <linux/interrupt.h> #include <linux/irqchip/chained_irq.h> #include <linux/init.h> +#include <linux/module.h> #include <linux/of_address.h> #include <linux/of_device.h> #include <linux/of_irq.h> @@ -705,6 +706,13 @@ static int altera_pcie_init_irq_domain(struct altera_pcie *pcie) return 0; } +static int altera_pcie_irq_teardown(struct altera_pcie *pcie) +{ + irq_set_chained_handler_and_data(pcie->irq, NULL, NULL); + irq_domain_remove(pcie->irq_domain); + irq_dispose_mapping(pcie->irq); +} + static int altera_pcie_parse_dt(struct altera_pcie *pcie) { struct device *dev = &pcie->pdev->dev; @@ -798,6 +806,7 @@ static int altera_pcie_probe(struct platform_device *pdev) pcie = pci_host_bridge_priv(bridge); pcie->pdev = pdev; + platform_set_drvdata(pdev, pcie); match = of_match_device(altera_pcie_of_match, &pdev->dev); if (!match) @@ -855,13 +864,28 @@ static int altera_pcie_probe(struct platform_device *pdev) return ret; } +static int altera_pcie_remove(struct platform_device *pdev) +{ + struct altera_pcie *pcie = platform_get_drvdata(pdev); + struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie); + + pci_stop_root_bus(bridge->bus); + pci_remove_root_bus(bridge->bus); + pci_free_resource_list(&pcie->resources); + altera_pcie_irq_teardown(pcie); + + return 0; +} + static struct platform_driver altera_pcie_driver = { .probe = altera_pcie_probe, + .remove = altera_pcie_remove, .driver = { .name = "altera-pcie", .of_match_table = altera_pcie_of_match, - .suppress_bind_attrs = true, }, }; -builtin_platform_driver(altera_pcie_driver); +MODULE_DEVICE_TABLE(of, altera_pcie_of_match); +module_platform_driver(altera_pcie_driver); +MODULE_LICENSE("GPL v2");
Altera PCIe Rootport IP is a soft IP and is only available after FPGA image is programmed. Make driver modulable to support use case FPGA image is programmed after kernel is booted. User proram FPGA image in kernel then only load PCIe driver module. Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> --- drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pcie-altera.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-)