* [PATCH 0/5] Modularize PCI_DW related drivers.
@ 2016-02-08 0:00 Paul Gortmaker
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
0 siblings, 2 replies; 11+ messages in thread
From: Paul Gortmaker @ 2016-02-08 0:00 UTC (permalink / raw)
To: linux-kernel
Cc: Paul Gortmaker, 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
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.
[1] https://lkml.kernel.org/r/20160108203102.GH5354@localhost
--
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: devicetree@vger.kernel.org
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Ley Foon Tan <lftan@altera.com>
Cc: Murali Karicheri <m-karicheri2@ti.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Paul Gortmaker (5):
ARM: add EXPORT_SYMBOL of hook_fault_code for PCI host modularization
drivers/of: add EXPORT_SYMBOL to of_irq_count
drivers/pci: export dw syms enabling board specific PCI code to be
tristate
drivers/pci: make host/pci-keystone-dw.c modular
drivers/pci: make most of the PCI_DW drivers modular
arch/arm/mm/fault.c | 3 ++-
drivers/of/irq.c | 1 +
drivers/pci/host/Kconfig | 16 ++++++++--------
drivers/pci/host/pci-keystone-dw.c | 16 +++++++++++++++-
drivers/pci/host/pcie-designware.c | 7 +++++++
5 files changed, 33 insertions(+), 10 deletions(-)
--
2.6.1
^ permalink raw reply [flat|nested] 11+ 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 ` 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
1 sibling, 1 reply; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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
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
1 sibling, 1 reply; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread
end of thread, other threads:[~2016-03-15 20:52 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-08 0:00 [PATCH 0/5] Modularize PCI_DW related drivers Paul Gortmaker
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
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
2016-02-25 8:35 ` Arnd Bergmann
2016-02-29 9:29 ` Kishon Vijay Abraham I
2016-03-01 21:35 ` Arnd Bergmann
2016-03-15 20:50 ` Murali Karicheri
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).