* [PATCH 3/5] drivers/pci: export dw syms enabling board specific PCI code to be tristate
2016-02-08 0:00 [PATCH 0/5] Modularize PCI_DW related drivers Paul Gortmaker
@ 2016-02-08 0:00 ` Paul Gortmaker
2016-02-08 9:57 ` Arnd Bergmann
2016-02-08 0:00 ` [PATCH 4/5] drivers/pci: make host/pci-keystone-dw.c modular Paul Gortmaker
` (2 subsequent siblings)
3 siblings, 1 reply; 18+ messages in thread
From: Paul Gortmaker @ 2016-02-08 0:00 UTC (permalink / raw)
To: linux-kernel
Cc: Paul Gortmaker, Jingoo Han, Pratyush Anand, Bjorn Helgaas,
Geert Uytterhoeven, Stanimir Varbanov, Thierry Reding,
Arnd Bergmann, linux-pci
After converting the drivers who select PCI_DW from bool to tristate,
the following symbols need to be exported, simply based on the output
from modpost failures.
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Pratyush Anand <pratyush.anand@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/pci/host/pcie-designware.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 21716827847a..65c0c81b8852 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -91,6 +91,7 @@ int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
return PCIBIOS_SUCCESSFUL;
}
+EXPORT_SYMBOL(dw_pcie_cfg_read);
int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val)
{
@@ -108,6 +109,7 @@ int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val)
return PCIBIOS_SUCCESSFUL;
}
+EXPORT_SYMBOL(dw_pcie_cfg_write);
static inline void dw_pcie_readl_rc(struct pcie_port *pp, u32 reg, u32 *val)
{
@@ -201,6 +203,7 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
return ret;
}
+EXPORT_SYMBOL(dw_handle_msi_irq);
void dw_pcie_msi_init(struct pcie_port *pp)
{
@@ -215,6 +218,7 @@ void dw_pcie_msi_init(struct pcie_port *pp)
dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4,
(u32)(msi_target >> 32 & 0xffffffff));
}
+EXPORT_SYMBOL(dw_pcie_msi_init);
static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
{
@@ -387,6 +391,7 @@ int dw_pcie_link_up(struct pcie_port *pp)
return 0;
}
+EXPORT_SYMBOL(dw_pcie_link_up);
static int dw_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
irq_hw_number_t hwirq)
@@ -562,6 +567,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
pci_bus_add_devices(bus);
return 0;
}
+EXPORT_SYMBOL(dw_pcie_host_init);
static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
u32 devfn, int where, int size, u32 *val)
@@ -771,6 +777,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
PCI_COMMAND_MASTER | PCI_COMMAND_SERR;
dw_pcie_writel_rc(pp, val, PCI_COMMAND);
}
+EXPORT_SYMBOL(dw_pcie_setup_rc);
MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
MODULE_DESCRIPTION("Designware PCIe host controller driver");
--
2.6.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] drivers/pci: export dw syms enabling board specific PCI code to be tristate
2016-02-08 0:00 ` [PATCH 3/5] drivers/pci: export dw syms enabling board specific PCI code to be tristate Paul Gortmaker
@ 2016-02-08 9:57 ` Arnd Bergmann
2016-02-08 15:53 ` Paul Gortmaker
0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2016-02-08 9:57 UTC (permalink / raw)
To: Paul Gortmaker
Cc: linux-kernel, Jingoo Han, Pratyush Anand, Bjorn Helgaas,
Geert Uytterhoeven, Stanimir Varbanov, Thierry Reding, linux-pci
On Sunday 07 February 2016 19:00:42 Paul Gortmaker wrote:
> After converting the drivers who select PCI_DW from bool to tristate,
> the following symbols need to be exported, simply based on the output
> from modpost failures.
>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Pratyush Anand <pratyush.anand@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-pci@vger.kernel.org
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>
Looks good in principle, but maybe use EXPORT_SYMBOL_GPL? These
are just meant to be used internally by the drivers after all.
Arnd
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] drivers/pci: export dw syms enabling board specific PCI code to be tristate
2016-02-08 9:57 ` Arnd Bergmann
@ 2016-02-08 15:53 ` Paul Gortmaker
0 siblings, 0 replies; 18+ messages in thread
From: Paul Gortmaker @ 2016-02-08 15:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, Jingoo Han, Pratyush Anand, Bjorn Helgaas,
Geert Uytterhoeven, Stanimir Varbanov, Thierry Reding, linux-pci
[Re: [PATCH 3/5] drivers/pci: export dw syms enabling board specific PCI code to be tristate] On 08/02/2016 (Mon 10:57) Arnd Bergmann wrote:
> On Sunday 07 February 2016 19:00:42 Paul Gortmaker wrote:
> > After converting the drivers who select PCI_DW from bool to tristate,
> > the following symbols need to be exported, simply based on the output
> > from modpost failures.
> >
> > Cc: Jingoo Han <jingoohan1@gmail.com>
> > Cc: Pratyush Anand <pratyush.anand@gmail.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: linux-pci@vger.kernel.org
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> >
>
> Looks good in principle, but maybe use EXPORT_SYMBOL_GPL? These
> are just meant to be used internally by the drivers after all.
Sure, I can make that restriction.
P.
--
>
> Arnd
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/5] drivers/pci: make host/pci-keystone-dw.c modular
2016-02-08 0:00 [PATCH 0/5] Modularize PCI_DW related drivers Paul Gortmaker
2016-02-08 0:00 ` [PATCH 3/5] drivers/pci: export dw syms enabling board specific PCI code to be tristate Paul Gortmaker
@ 2016-02-08 0:00 ` Paul Gortmaker
2016-02-08 9:59 ` Arnd Bergmann
2016-02-08 0:00 ` [PATCH 5/5] drivers/pci: make most of the PCI_DW drivers modular Paul Gortmaker
2016-02-24 6:09 ` [PATCH 0/5] Modularize PCI_DW related drivers Kishon Vijay Abraham I
3 siblings, 1 reply; 18+ messages in thread
From: Paul Gortmaker @ 2016-02-08 0:00 UTC (permalink / raw)
To: linux-kernel
Cc: Paul Gortmaker, Murali Karicheri, Bjorn Helgaas,
Stanimir Varbanov, Geert Uytterhoeven, Thierry Reding,
Arnd Bergmann, linux-pci, linux-arm-kernel
Export the symbols that this driver requires in order for it to
be modular. In addition to the one use case of a dw_pci sym, it
has many instances of its own ks_dw_pci syms that need exporting
in order to modpost w/o error.
Cc: Murali Karicheri <m-karicheri2@ti.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/pci/host/Kconfig | 2 +-
drivers/pci/host/pci-keystone-dw.c | 16 +++++++++++++++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 75a605426538..b040ad7ba44d 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -69,7 +69,7 @@ config PCIE_SPEAR13XX
Say Y here if you want PCIe support on SPEAr13XX SoCs.
config PCI_KEYSTONE
- bool "TI Keystone PCIe controller"
+ tristate "TI Keystone PCIe controller"
depends on ARCH_KEYSTONE
select PCIE_DW
select PCIEPORTBUS
diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
index ed34c9520a02..f1550a8d2e29 100644
--- a/drivers/pci/host/pci-keystone-dw.c
+++ b/drivers/pci/host/pci-keystone-dw.c
@@ -76,6 +76,7 @@ phys_addr_t ks_dw_pcie_get_msi_addr(struct pcie_port *pp)
return ks_pcie->app.start + MSI_IRQ;
}
+EXPORT_SYMBOL(ks_dw_pcie_get_msi_addr);
void ks_dw_pcie_handle_msi_irq(struct keystone_pcie *ks_pcie, int offset)
{
@@ -99,6 +100,7 @@ void ks_dw_pcie_handle_msi_irq(struct keystone_pcie *ks_pcie, int offset)
}
}
}
+EXPORT_SYMBOL(ks_dw_pcie_handle_msi_irq);
static void ks_dw_pcie_msi_irq_ack(struct irq_data *d)
{
@@ -127,6 +129,7 @@ void ks_dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
writel(BIT(bit_pos),
ks_pcie->va_app_base + MSI0_IRQ_ENABLE_SET + (reg_offset << 4));
}
+EXPORT_SYMBOL(ks_dw_pcie_msi_set_irq);
void ks_dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
{
@@ -137,6 +140,7 @@ void ks_dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
writel(BIT(bit_pos),
ks_pcie->va_app_base + MSI0_IRQ_ENABLE_CLR + (reg_offset << 4));
}
+EXPORT_SYMBOL(ks_dw_pcie_msi_clear_irq);
static void ks_dw_pcie_msi_irq_mask(struct irq_data *d)
{
@@ -220,6 +224,7 @@ int ks_dw_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
return 0;
}
+EXPORT_SYMBOL(ks_dw_pcie_msi_host_init);
void ks_dw_pcie_enable_legacy_irqs(struct keystone_pcie *ks_pcie)
{
@@ -228,6 +233,7 @@ void ks_dw_pcie_enable_legacy_irqs(struct keystone_pcie *ks_pcie)
for (i = 0; i < MAX_LEGACY_IRQS; i++)
writel(0x1, ks_pcie->va_app_base + IRQ_ENABLE_SET + (i << 4));
}
+EXPORT_SYMBOL(ks_dw_pcie_enable_legacy_irqs);
void ks_dw_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie, int offset)
{
@@ -247,6 +253,7 @@ void ks_dw_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie, int offset)
/* EOI the INTx interrupt */
writel(offset, ks_pcie->va_app_base + IRQ_EOI);
}
+EXPORT_SYMBOL(ks_dw_pcie_handle_legacy_irq);
static void ks_dw_pcie_ack_legacy_irq(struct irq_data *d)
{
@@ -347,6 +354,7 @@ void ks_dw_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
writel(OB_XLAT_EN_VAL | readl(ks_pcie->va_app_base + CMD_STATUS),
ks_pcie->va_app_base + CMD_STATUS);
}
+EXPORT_SYMBOL(ks_dw_pcie_setup_rc_app_regs);
/**
* ks_pcie_cfg_setup() - Set up configuration space address for a device
@@ -400,6 +408,7 @@ int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
return dw_pcie_cfg_read(addr + where, size, val);
}
+EXPORT_SYMBOL(ks_dw_pcie_rd_other_conf);
int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
unsigned int devfn, int where, int size, u32 val)
@@ -412,6 +421,7 @@ int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
return dw_pcie_cfg_write(addr + where, size, val);
}
+EXPORT_SYMBOL(ks_dw_pcie_wr_other_conf);
/**
* ks_dw_pcie_v3_65_scan_bus() - keystone scan_bus post initialization
@@ -437,6 +447,7 @@ void ks_dw_pcie_v3_65_scan_bus(struct pcie_port *pp)
*/
writel(ks_pcie->app.start, pp->dbi_base + PCI_BASE_ADDRESS_0);
}
+EXPORT_SYMBOL(ks_dw_pcie_v3_65_scan_bus);
/**
* ks_dw_pcie_link_up() - Check if link up
@@ -447,6 +458,7 @@ int ks_dw_pcie_link_up(struct pcie_port *pp)
return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0;
}
+EXPORT_SYMBOL(ks_dw_pcie_link_up);
void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
{
@@ -461,6 +473,7 @@ void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
val = readl(ks_pcie->va_app_base + CMD_STATUS);
writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);
}
+EXPORT_SYMBOL(ks_dw_pcie_initiate_link_train);
/**
* ks_dw_pcie_host_init() - initialize host for v3_65 dw hardware
@@ -469,7 +482,7 @@ void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
* and call dw_pcie_v3_65_host_init() API to initialize the Keystone
* PCI host controller.
*/
-int __init ks_dw_pcie_host_init(struct keystone_pcie *ks_pcie,
+int ks_dw_pcie_host_init(struct keystone_pcie *ks_pcie,
struct device_node *msi_intc_np)
{
struct pcie_port *pp = &ks_pcie->pp;
@@ -510,3 +523,4 @@ int __init ks_dw_pcie_host_init(struct keystone_pcie *ks_pcie,
return dw_pcie_host_init(pp);
}
+EXPORT_SYMBOL(ks_dw_pcie_host_init);
--
2.6.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] drivers/pci: make host/pci-keystone-dw.c modular
2016-02-08 0:00 ` [PATCH 4/5] drivers/pci: make host/pci-keystone-dw.c modular Paul Gortmaker
@ 2016-02-08 9:59 ` Arnd Bergmann
2016-02-08 15:53 ` Paul Gortmaker
0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2016-02-08 9:59 UTC (permalink / raw)
To: Paul Gortmaker
Cc: linux-kernel, Murali Karicheri, Bjorn Helgaas, Stanimir Varbanov,
Geert Uytterhoeven, Thierry Reding, linux-pci, linux-arm-kernel
On Sunday 07 February 2016 19:00:43 Paul Gortmaker wrote:
> Export the symbols that this driver requires in order for it to
> be modular. In addition to the one use case of a dw_pci sym, it
> has many instances of its own ks_dw_pci syms that need exporting
> in order to modpost w/o error.
>
> Cc: Murali Karicheri <m-karicheri2@ti.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
Could we just have one module for the driver instead of two?
Something like below (untested)
Arnd
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 7b2f20c6ccc6..0b472c680348 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -8,7 +8,8 @@ obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
-obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
+obj-$(CONFIG_PCI_KEYSTONE) pci-keystone-mod.o
+pci-keystone-mod-objs := pci-keystone-dw.o pci-keystone.o
obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] drivers/pci: make host/pci-keystone-dw.c modular
2016-02-08 9:59 ` Arnd Bergmann
@ 2016-02-08 15:53 ` Paul Gortmaker
2016-02-08 19:03 ` Murali Karicheri
0 siblings, 1 reply; 18+ messages in thread
From: Paul Gortmaker @ 2016-02-08 15:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, Murali Karicheri, Bjorn Helgaas, Stanimir Varbanov,
Geert Uytterhoeven, Thierry Reding, linux-pci, linux-arm-kernel
[Re: [PATCH 4/5] drivers/pci: make host/pci-keystone-dw.c modular] On 08/02/2016 (Mon 10:59) Arnd Bergmann wrote:
> On Sunday 07 February 2016 19:00:43 Paul Gortmaker wrote:
> > Export the symbols that this driver requires in order for it to
> > be modular. In addition to the one use case of a dw_pci sym, it
> > has many instances of its own ks_dw_pci syms that need exporting
> > in order to modpost w/o error.
> >
> > Cc: Murali Karicheri <m-karicheri2@ti.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: linux-pci@vger.kernel.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> > ---
>
>
> Could we just have one module for the driver instead of two?
That thought crossed my mind, but I was considering merging the C files
and figured it might be more churn than the maintainer wanted. If
maintainers are good with the below (assuming it works) then yes that
also seems reasonable.
P.
--
>
> Something like below (untested)
>
> Arnd
>
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 7b2f20c6ccc6..0b472c680348 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -8,7 +8,8 @@ obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
> obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
> obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> -obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
> +obj-$(CONFIG_PCI_KEYSTONE) pci-keystone-mod.o
> +pci-keystone-mod-objs := pci-keystone-dw.o pci-keystone.o
> obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
> obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
> obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] drivers/pci: make host/pci-keystone-dw.c modular
2016-02-08 15:53 ` Paul Gortmaker
@ 2016-02-08 19:03 ` Murali Karicheri
0 siblings, 0 replies; 18+ messages in thread
From: Murali Karicheri @ 2016-02-08 19:03 UTC (permalink / raw)
To: Paul Gortmaker, Arnd Bergmann
Cc: linux-kernel, Bjorn Helgaas, Stanimir Varbanov,
Geert Uytterhoeven, Thierry Reding, linux-pci, linux-arm-kernel
On 02/08/2016 10:53 AM, Paul Gortmaker wrote:
> [Re: [PATCH 4/5] drivers/pci: make host/pci-keystone-dw.c modular] On 08/02/2016 (Mon 10:59) Arnd Bergmann wrote:
>
>> On Sunday 07 February 2016 19:00:43 Paul Gortmaker wrote:
>>> Export the symbols that this driver requires in order for it to
>>> be modular. In addition to the one use case of a dw_pci sym, it
>>> has many instances of its own ks_dw_pci syms that need exporting
>>> in order to modpost w/o error.
>>>
>>> Cc: Murali Karicheri <m-karicheri2@ti.com>
>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>> Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: linux-pci@vger.kernel.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>>> ---
>>
>>
>> Could we just have one module for the driver instead of two?
I agree. Please note that Keystone PCI is tested only as static.
I can help test the module if this series get to the next revision,
but it might be a while before I can do the test.
Murali
>
> That thought crossed my mind, but I was considering merging the C files
> and figured it might be more churn than the maintainer wanted. If
> maintainers are good with the below (assuming it works) then yes that
> also seems reasonable.
>
> P.
> --
>
>>
>> Something like below (untested)
>>
>> Arnd
>>
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 7b2f20c6ccc6..0b472c680348 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -8,7 +8,8 @@ obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
>> obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
>> obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
>> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>> -obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>> +obj-$(CONFIG_PCI_KEYSTONE) pci-keystone-mod.o
>> +pci-keystone-mod-objs := pci-keystone-dw.o pci-keystone.o
>> obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
>> obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
>> obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
>>
>
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 5/5] drivers/pci: make most of the PCI_DW drivers modular
2016-02-08 0:00 [PATCH 0/5] Modularize PCI_DW related drivers Paul Gortmaker
2016-02-08 0:00 ` [PATCH 3/5] drivers/pci: export dw syms enabling board specific PCI code to be tristate Paul Gortmaker
2016-02-08 0:00 ` [PATCH 4/5] drivers/pci: make host/pci-keystone-dw.c modular Paul Gortmaker
@ 2016-02-08 0:00 ` Paul Gortmaker
2016-02-08 10:00 ` Arnd Bergmann
2016-02-24 6:09 ` [PATCH 0/5] Modularize PCI_DW related drivers Kishon Vijay Abraham I
3 siblings, 1 reply; 18+ messages in thread
From: Paul Gortmaker @ 2016-02-08 0:00 UTC (permalink / raw)
To: linux-kernel
Cc: Paul Gortmaker, Bjorn Helgaas, Ley Foon Tan, Geert Uytterhoeven,
Stanimir Varbanov, Thierry Reding, Arnd Bergmann, linux-pci,
linux-arm-msm
We've exported the symbols that we know these specific drivers
will need as tristate, so now we can make the conversion from
bool to tristate w/o concern for build regressions.
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ley Foon Tan <lftan@altera.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/pci/host/Kconfig | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index b040ad7ba44d..f3020389ddaa 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -2,7 +2,7 @@ menu "PCI host controller drivers"
depends on PCI
config PCI_DRA7XX
- bool "TI DRA7xx PCIe controller"
+ tristate "TI DRA7xx PCIe controller"
select PCIE_DW
depends on OF && HAS_IOMEM && TI_PIPE3
depends on BROKEN
@@ -20,13 +20,13 @@ config PCIE_DW
bool
config PCI_EXYNOS
- bool "Samsung Exynos PCIe controller"
+ tristate "Samsung Exynos PCIe controller"
depends on SOC_EXYNOS5440
select PCIEPORTBUS
select PCIE_DW
config PCI_IMX6
- bool "Freescale i.MX6 PCIe controller"
+ tristate "Freescale i.MX6 PCIe controller"
depends on SOC_IMX6Q
select PCIEPORTBUS
select PCIE_DW
@@ -61,7 +61,7 @@ config PCI_HOST_GENERIC
controller, such as the one emulated by kvmtool.
config PCIE_SPEAR13XX
- bool "STMicroelectronics SPEAr PCIe controller"
+ tristate "STMicroelectronics SPEAr PCIe controller"
depends on ARCH_SPEAR13XX
select PCIEPORTBUS
select PCIE_DW
@@ -106,7 +106,7 @@ config PCI_XGENE_MSI
This MSI driver supports 5 PCIe ports on the APM X-Gene v1 SoC.
config PCI_LAYERSCAPE
- bool "Freescale Layerscape PCIe controller"
+ tristate "Freescale Layerscape PCIe controller"
depends on OF && (ARM || ARCH_LAYERSCAPE)
select PCIE_DW
select MFD_SYSCON
@@ -174,7 +174,7 @@ config PCIE_ALTERA_MSI
config PCI_HISI
depends on OF && ARM64
- bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
+ tristate "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
select PCIEPORTBUS
select PCIE_DW
help
@@ -182,7 +182,7 @@ config PCI_HISI
Hip05 and Hip06 SoCs
config PCIE_QCOM
- bool "Qualcomm PCIe controller"
+ tristate "Qualcomm PCIe controller"
depends on ARCH_QCOM && OF
select PCIE_DW
select PCIEPORTBUS
--
2.6.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] drivers/pci: make most of the PCI_DW drivers modular
2016-02-08 0:00 ` [PATCH 5/5] drivers/pci: make most of the PCI_DW drivers modular Paul Gortmaker
@ 2016-02-08 10:00 ` Arnd Bergmann
2016-02-08 15:51 ` Paul Gortmaker
0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2016-02-08 10:00 UTC (permalink / raw)
To: Paul Gortmaker
Cc: linux-kernel, Bjorn Helgaas, Ley Foon Tan, Geert Uytterhoeven,
Stanimir Varbanov, Thierry Reding, linux-pci, linux-arm-msm
On Sunday 07 February 2016 19:00:44 Paul Gortmaker wrote:
> We've exported the symbols that we know these specific drivers
> will need as tristate, so now we can make the conversion from
> bool to tristate w/o concern for build regressions.
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Ley Foon Tan <lftan@altera.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-msm@vger.kernel.org
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Looks good, but I'm not sure what happens on unload. Did you check?
Until this is tested, maybe it's best to prevent module unloading?
Arnd
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] drivers/pci: make most of the PCI_DW drivers modular
2016-02-08 10:00 ` Arnd Bergmann
@ 2016-02-08 15:51 ` Paul Gortmaker
0 siblings, 0 replies; 18+ messages in thread
From: Paul Gortmaker @ 2016-02-08 15:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, Bjorn Helgaas, Ley Foon Tan, Geert Uytterhoeven,
Stanimir Varbanov, Thierry Reding, linux-pci, linux-arm-msm
[Re: [PATCH 5/5] drivers/pci: make most of the PCI_DW drivers modular] On 08/02/2016 (Mon 11:00) Arnd Bergmann wrote:
> On Sunday 07 February 2016 19:00:44 Paul Gortmaker wrote:
> > We've exported the symbols that we know these specific drivers
> > will need as tristate, so now we can make the conversion from
> > bool to tristate w/o concern for build regressions.
> >
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Ley Foon Tan <lftan@altera.com>
> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: linux-pci@vger.kernel.org
> > Cc: linux-arm-msm@vger.kernel.org
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>
> Looks good, but I'm not sure what happens on unload. Did you check?
Per the 0/N I've not got the capability to do run time testing for any
of this stuff. And from previous discussions, ISTR that people didn't
want to see a blanket block-unload policy implemented.
> Until this is tested, maybe it's best to prevent module unloading?
If that is what people want, I can add that.
P.
--
>
> Arnd
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] Modularize PCI_DW related drivers.
2016-02-08 0:00 [PATCH 0/5] Modularize PCI_DW related drivers Paul Gortmaker
` (2 preceding siblings ...)
2016-02-08 0:00 ` [PATCH 5/5] drivers/pci: make most of the PCI_DW drivers modular Paul Gortmaker
@ 2016-02-24 6:09 ` Kishon Vijay Abraham I
2016-02-24 9:04 ` Arnd Bergmann
3 siblings, 1 reply; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-24 6:09 UTC (permalink / raw)
To: Paul Gortmaker, linux-kernel
Cc: Arnd Bergmann, Bjorn Helgaas, devicetree, Frank Rowand,
Geert Uytterhoeven, Grant Likely, Ley Foon Tan, Murali Karicheri,
Rob Herring, Russell King, Stanimir Varbanov, Thierry Reding,
linux-arm-kernel, linux-arm-msm, linux-pci
Hi,
On Monday 08 February 2016 05:30 AM, Paul Gortmaker wrote:
> In a recent patch series that aimed to remove code related to module
> unload for PCI support that was simply non modular, the discussion
> led to people wanting to keep the code and push towards taking the
> steps needed to support moving it towards tristate instead[1].
>
> Here, we take step one, which is simply making the Kconfig change
> and then dealing with any build fallout or modpost fallout. What
> amounts to essentially a sanity build test. To be clear, these
> have not been runtime validated; that will need to be done by those
> with access to real hardware. However, the changes are not anything
> that should disrupt any existing built-in validation, so real world
> users should not be impacted by this change.
>
> We start with a smaller family of drivers; those that actively select
> PCI_DW, as a nice self contained group to test the waters and see if
> everyone is still good with this approach before investing more time
> on a wider scale to other pci/host/ code blocks.
>
> As such the drivers here share a dependency on having the same group
> of functions exported in order to successfully complete modpost.
>
> In addition, we have to stray outside drivers/pci to add exports
> in two places; once for an ARM fault handler, and once for an OF
> variable.
>
> The pci-keystone-dw.c instance was handled separately because it
> consists of two source files that need their own group of driver
> specific exports above and beyond the "shared" ones.
>
> Then we convert the Kconfig for all remaining at once; we could have
> done it on a per driver basis for ease of revert if anyone really
> objects, but since it would be a one line change, that seemed like
> not a real concern.
>
> Build testing was done on the linux-next tree for arm allmodconfig.
I took these patches and gave a test with DRA7xx board. As expected there was
no issues when the driver was built-in. However when I tried to rmmod/modprobe
I got this error [2].
Thanks
Kishon
[2] -> http://pastebin.ubuntu.com/15185894/
>
> [1] https://lkml.kernel.org/r/20160108203102.GH5354@localhost
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] Modularize PCI_DW related drivers.
2016-02-24 6:09 ` [PATCH 0/5] Modularize PCI_DW related drivers Kishon Vijay Abraham I
@ 2016-02-24 9:04 ` Arnd Bergmann
2016-02-25 8:13 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2016-02-24 9:04 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Paul Gortmaker, linux-kernel, Bjorn Helgaas, devicetree,
Frank Rowand, Geert Uytterhoeven, Grant Likely, Ley Foon Tan,
Murali Karicheri, Rob Herring, Russell King, Stanimir Varbanov,
Thierry Reding, linux-arm-kernel, linux-arm-msm, linux-pci
On Wednesday 24 February 2016 11:39:26 Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 08 February 2016 05:30 AM, Paul Gortmaker wrote:
> > In a recent patch series that aimed to remove code related to module
> > unload for PCI support that was simply non modular, the discussion
> > led to people wanting to keep the code and push towards taking the
> > steps needed to support moving it towards tristate instead[1].
> >
> > Here, we take step one, which is simply making the Kconfig change
> > and then dealing with any build fallout or modpost fallout. What
> > amounts to essentially a sanity build test. To be clear, these
> > have not been runtime validated; that will need to be done by those
> > with access to real hardware. However, the changes are not anything
> > that should disrupt any existing built-in validation, so real world
> > users should not be impacted by this change.
> >
> > We start with a smaller family of drivers; those that actively select
> > PCI_DW, as a nice self contained group to test the waters and see if
> > everyone is still good with this approach before investing more time
> > on a wider scale to other pci/host/ code blocks.
> >
> > As such the drivers here share a dependency on having the same group
> > of functions exported in order to successfully complete modpost.
> >
> > In addition, we have to stray outside drivers/pci to add exports
> > in two places; once for an ARM fault handler, and once for an OF
> > variable.
> >
> > The pci-keystone-dw.c instance was handled separately because it
> > consists of two source files that need their own group of driver
> > specific exports above and beyond the "shared" ones.
> >
> > Then we convert the Kconfig for all remaining at once; we could have
> > done it on a per driver basis for ease of revert if anyone really
> > objects, but since it would be a one line change, that seemed like
> > not a real concern.
> >
> > Build testing was done on the linux-next tree for arm allmodconfig.
>
> I took these patches and gave a test with DRA7xx board. As expected there was
> no issues when the driver was built-in. However when I tried to rmmod/modprobe
> I got this error [2].
Thanks for testing this!
> [2] -> http://pastebin.ubuntu.com/15185894/
It looks like you are hitting the BUG_ON() in ioremap_pte_range()
that checks if a virtual address already has a page table entry,
which in turn is probably a result of dw_pcie_host_init()
calling pci_remap_iospace() again for the same memory area
it has called the last time, and no cleanup done inbetween.
Could you try adding a pci_unmap_iospace() and calling that
in the device remove function? Let me know if you need help
implementing it.
Arnd
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] Modularize PCI_DW related drivers.
2016-02-24 9:04 ` Arnd Bergmann
@ 2016-02-25 8:13 ` Kishon Vijay Abraham I
2016-02-25 8:35 ` Arnd Bergmann
0 siblings, 1 reply; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-25 8:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Paul Gortmaker, linux-kernel, Bjorn Helgaas, devicetree,
Frank Rowand, Geert Uytterhoeven, Grant Likely, Ley Foon Tan,
Murali Karicheri, Rob Herring, Russell King, Stanimir Varbanov,
Thierry Reding, linux-arm-kernel, linux-arm-msm, linux-pci
Hi Arnd,
On Wednesday 24 February 2016 02:34 PM, Arnd Bergmann wrote:
> On Wednesday 24 February 2016 11:39:26 Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Monday 08 February 2016 05:30 AM, Paul Gortmaker wrote:
>>> In a recent patch series that aimed to remove code related to module
>>> unload for PCI support that was simply non modular, the discussion
>>> led to people wanting to keep the code and push towards taking the
>>> steps needed to support moving it towards tristate instead[1].
>>>
>>> Here, we take step one, which is simply making the Kconfig change
>>> and then dealing with any build fallout or modpost fallout. What
>>> amounts to essentially a sanity build test. To be clear, these
>>> have not been runtime validated; that will need to be done by those
>>> with access to real hardware. However, the changes are not anything
>>> that should disrupt any existing built-in validation, so real world
>>> users should not be impacted by this change.
>>>
>>> We start with a smaller family of drivers; those that actively select
>>> PCI_DW, as a nice self contained group to test the waters and see if
>>> everyone is still good with this approach before investing more time
>>> on a wider scale to other pci/host/ code blocks.
>>>
>>> As such the drivers here share a dependency on having the same group
>>> of functions exported in order to successfully complete modpost.
>>>
>>> In addition, we have to stray outside drivers/pci to add exports
>>> in two places; once for an ARM fault handler, and once for an OF
>>> variable.
>>>
>>> The pci-keystone-dw.c instance was handled separately because it
>>> consists of two source files that need their own group of driver
>>> specific exports above and beyond the "shared" ones.
>>>
>>> Then we convert the Kconfig for all remaining at once; we could have
>>> done it on a per driver basis for ease of revert if anyone really
>>> objects, but since it would be a one line change, that seemed like
>>> not a real concern.
>>>
>>> Build testing was done on the linux-next tree for arm allmodconfig.
>>
>> I took these patches and gave a test with DRA7xx board. As expected there was
>> no issues when the driver was built-in. However when I tried to rmmod/modprobe
>> I got this error [2].
>
> Thanks for testing this!
>
>> [2] -> http://pastebin.ubuntu.com/15185894/
>
> It looks like you are hitting the BUG_ON() in ioremap_pte_range()
> that checks if a virtual address already has a page table entry,
> which in turn is probably a result of dw_pcie_host_init()
> calling pci_remap_iospace() again for the same memory area
> it has called the last time, and no cleanup done inbetween.
>
> Could you try adding a pci_unmap_iospace() and calling that
> in the device remove function? Let me know if you need help
> implementing it.
That didn't look straight forward to me :-( I'll try to see this next week. Any
help from you will make it simpler for me.
Thanks
Kishon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] Modularize PCI_DW related drivers.
2016-02-25 8:13 ` Kishon Vijay Abraham I
@ 2016-02-25 8:35 ` Arnd Bergmann
2016-02-29 9:29 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2016-02-25 8:35 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Paul Gortmaker, linux-kernel, Bjorn Helgaas, devicetree,
Frank Rowand, Geert Uytterhoeven, Grant Likely, Ley Foon Tan,
Murali Karicheri, Rob Herring, Russell King, Stanimir Varbanov,
Thierry Reding, linux-arm-kernel, linux-arm-msm, linux-pci
On Thursday 25 February 2016 13:43:48 Kishon Vijay Abraham I wrote:
> Hi Arnd,
>
> On Wednesday 24 February 2016 02:34 PM, Arnd Bergmann wrote:
> > On Wednesday 24 February 2016 11:39:26 Kishon Vijay Abraham I wrote:
> >> Hi,
> >>
> >> On Monday 08 February 2016 05:30 AM, Paul Gortmaker wrote:
> >>> In a recent patch series that aimed to remove code related to module
> >>> unload for PCI support that was simply non modular, the discussion
> >>> led to people wanting to keep the code and push towards taking the
> >>> steps needed to support moving it towards tristate instead[1].
> >>>
> >>> Here, we take step one, which is simply making the Kconfig change
> >>> and then dealing with any build fallout or modpost fallout. What
> >>> amounts to essentially a sanity build test. To be clear, these
> >>> have not been runtime validated; that will need to be done by those
> >>> with access to real hardware. However, the changes are not anything
> >>> that should disrupt any existing built-in validation, so real world
> >>> users should not be impacted by this change.
> >>>
> >>> We start with a smaller family of drivers; those that actively select
> >>> PCI_DW, as a nice self contained group to test the waters and see if
> >>> everyone is still good with this approach before investing more time
> >>> on a wider scale to other pci/host/ code blocks.
> >>>
> >>> As such the drivers here share a dependency on having the same group
> >>> of functions exported in order to successfully complete modpost.
> >>>
> >>> In addition, we have to stray outside drivers/pci to add exports
> >>> in two places; once for an ARM fault handler, and once for an OF
> >>> variable.
> >>>
> >>> The pci-keystone-dw.c instance was handled separately because it
> >>> consists of two source files that need their own group of driver
> >>> specific exports above and beyond the "shared" ones.
> >>>
> >>> Then we convert the Kconfig for all remaining at once; we could have
> >>> done it on a per driver basis for ease of revert if anyone really
> >>> objects, but since it would be a one line change, that seemed like
> >>> not a real concern.
> >>>
> >>> Build testing was done on the linux-next tree for arm allmodconfig.
> >>
> >> I took these patches and gave a test with DRA7xx board. As expected there was
> >> no issues when the driver was built-in. However when I tried to rmmod/modprobe
> >> I got this error [2].
> >
> > Thanks for testing this!
> >
> >> [2] -> http://pastebin.ubuntu.com/15185894/
> >
> > It looks like you are hitting the BUG_ON() in ioremap_pte_range()
> > that checks if a virtual address already has a page table entry,
> > which in turn is probably a result of dw_pcie_host_init()
> > calling pci_remap_iospace() again for the same memory area
> > it has called the last time, and no cleanup done inbetween.
> >
> > Could you try adding a pci_unmap_iospace() and calling that
> > in the device remove function? Let me know if you need help
> > implementing it.
>
> That didn't look straight forward to me :-( I'll try to see this next week. Any
> help from you will make it simpler for me.
I tried writing the function now, and I think it's actually quite easy:
void pci_unmap_iospace(const struct resource *res)
{
#if defined(PCI_IOBASE) && defined(CONFIG_MMU)
return iounmap(PCI_IOBASE + res->start);
#endif
}
You just need to pass the same resource in here htat you pass into
pci_remap_iospace().
Arnd
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] Modularize PCI_DW related drivers.
2016-02-25 8:35 ` Arnd Bergmann
@ 2016-02-29 9:29 ` Kishon Vijay Abraham I
2016-03-01 21:35 ` Arnd Bergmann
0 siblings, 1 reply; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-29 9:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Paul Gortmaker, linux-kernel, Bjorn Helgaas, devicetree,
Frank Rowand, Geert Uytterhoeven, Grant Likely, Ley Foon Tan,
Murali Karicheri, Rob Herring, Russell King, Stanimir Varbanov,
Thierry Reding, linux-arm-kernel, linux-arm-msm, linux-pci
Hi Arnd,
On Thursday 25 February 2016 02:05 PM, Arnd Bergmann wrote:
> On Thursday 25 February 2016 13:43:48 Kishon Vijay Abraham I wrote:
>> Hi Arnd,
>>
>> On Wednesday 24 February 2016 02:34 PM, Arnd Bergmann wrote:
>>> On Wednesday 24 February 2016 11:39:26 Kishon Vijay Abraham I wrote:
>>>> Hi,
>>>>
>>>> On Monday 08 February 2016 05:30 AM, Paul Gortmaker wrote:
>>>>> In a recent patch series that aimed to remove code related to module
>>>>> unload for PCI support that was simply non modular, the discussion
>>>>> led to people wanting to keep the code and push towards taking the
>>>>> steps needed to support moving it towards tristate instead[1].
>>>>>
>>>>> Here, we take step one, which is simply making the Kconfig change
>>>>> and then dealing with any build fallout or modpost fallout. What
>>>>> amounts to essentially a sanity build test. To be clear, these
>>>>> have not been runtime validated; that will need to be done by those
>>>>> with access to real hardware. However, the changes are not anything
>>>>> that should disrupt any existing built-in validation, so real world
>>>>> users should not be impacted by this change.
>>>>>
>>>>> We start with a smaller family of drivers; those that actively select
>>>>> PCI_DW, as a nice self contained group to test the waters and see if
>>>>> everyone is still good with this approach before investing more time
>>>>> on a wider scale to other pci/host/ code blocks.
>>>>>
>>>>> As such the drivers here share a dependency on having the same group
>>>>> of functions exported in order to successfully complete modpost.
>>>>>
>>>>> In addition, we have to stray outside drivers/pci to add exports
>>>>> in two places; once for an ARM fault handler, and once for an OF
>>>>> variable.
>>>>>
>>>>> The pci-keystone-dw.c instance was handled separately because it
>>>>> consists of two source files that need their own group of driver
>>>>> specific exports above and beyond the "shared" ones.
>>>>>
>>>>> Then we convert the Kconfig for all remaining at once; we could have
>>>>> done it on a per driver basis for ease of revert if anyone really
>>>>> objects, but since it would be a one line change, that seemed like
>>>>> not a real concern.
>>>>>
>>>>> Build testing was done on the linux-next tree for arm allmodconfig.
>>>>
>>>> I took these patches and gave a test with DRA7xx board. As expected there was
>>>> no issues when the driver was built-in. However when I tried to rmmod/modprobe
>>>> I got this error [2].
>>>
>>> Thanks for testing this!
>>>
>>>> [2] -> http://pastebin.ubuntu.com/15185894/
>>>
>>> It looks like you are hitting the BUG_ON() in ioremap_pte_range()
>>> that checks if a virtual address already has a page table entry,
>>> which in turn is probably a result of dw_pcie_host_init()
>>> calling pci_remap_iospace() again for the same memory area
>>> it has called the last time, and no cleanup done inbetween.
>>>
>>> Could you try adding a pci_unmap_iospace() and calling that
>>> in the device remove function? Let me know if you need help
>>> implementing it.
>>
>> That didn't look straight forward to me :-( I'll try to see this next week. Any
>> help from you will make it simpler for me.
>
> I tried writing the function now, and I think it's actually quite easy:
>
> void pci_unmap_iospace(const struct resource *res)
> {
> #if defined(PCI_IOBASE) && defined(CONFIG_MMU)
> return iounmap(PCI_IOBASE + res->start);
> #endif
> }
>
> You just need to pass the same resource in here htat you pass into
> pci_remap_iospace().
I still seem to get the abort in ioremap_page_range().
Here's the patch I used [3] and here's the kernel log [4].
[3] -> http://pastebin.ubuntu.com/15241614/
[4] -> http://pastebin.ubuntu.com/15241637/
Thanks
Kishon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] Modularize PCI_DW related drivers.
2016-02-29 9:29 ` Kishon Vijay Abraham I
@ 2016-03-01 21:35 ` Arnd Bergmann
2016-03-15 20:50 ` Murali Karicheri
0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2016-03-01 21:35 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Paul Gortmaker, linux-kernel, Bjorn Helgaas, devicetree,
Frank Rowand, Geert Uytterhoeven, Grant Likely, Ley Foon Tan,
Murali Karicheri, Rob Herring, Russell King, Stanimir Varbanov,
Thierry Reding, linux-arm-kernel, linux-arm-msm, linux-pci
On Monday 29 February 2016 14:59:35 Kishon Vijay Abraham I wrote:
> > }
> >
> > You just need to pass the same resource in here htat you pass into
> > pci_remap_iospace().
>
> I still seem to get the abort in ioremap_page_range().
>
> Here's the patch I used [3] and here's the kernel log [4].
>
> [3] -> http://pastebin.ubuntu.com/15241614/
> [4] -> http://pastebin.ubuntu.com/15241637/
>
>
Sorry, I'm out of ideas here. The patch looks right to me, but the problem
looks unchanged.
Arnd
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] Modularize PCI_DW related drivers.
2016-03-01 21:35 ` Arnd Bergmann
@ 2016-03-15 20:50 ` Murali Karicheri
0 siblings, 0 replies; 18+ messages in thread
From: Murali Karicheri @ 2016-03-15 20:50 UTC (permalink / raw)
To: Arnd Bergmann, Kishon Vijay Abraham I
Cc: Paul Gortmaker, linux-kernel, Bjorn Helgaas, devicetree,
Frank Rowand, Geert Uytterhoeven, Grant Likely, Ley Foon Tan,
Rob Herring, Russell King, Stanimir Varbanov, Thierry Reding,
linux-arm-kernel, linux-arm-msm, linux-pci
On 03/01/2016 04:35 PM, Arnd Bergmann wrote:
> On Monday 29 February 2016 14:59:35 Kishon Vijay Abraham I wrote:
>>> }
>>>
>>> You just need to pass the same resource in here htat you pass into
>>> pci_remap_iospace().
>>
>> I still seem to get the abort in ioremap_page_range().
>>
>> Here's the patch I used [3] and here's the kernel log [4].
>>
>> [3] -> http://pastebin.ubuntu.com/15241614/
>> [4] -> http://pastebin.ubuntu.com/15241637/
>>
>>
>
> Sorry, I'm out of ideas here. The patch looks right to me, but the problem
> looks unchanged.
>
> Arnd
>
Was there any progress since this last response from Arnd? or is it a TBD?
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply [flat|nested] 18+ messages in thread