| Message ID | 20220517125058.18488-3-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: dwc: Various fixes and cleanups | expand |
On Tue, May 17, 2022 at 03:50:47PM +0300, Serge Semin wrote: > Commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") replaced > the locally defined DW PCIe host controller config-space accessors with > the generic methods pci_generic_config_read() and > pci_generic_config_write(). It was intended that the corresponding > bus-mapping callback returned a correct virtual address of the passed PCI > config-space register. The problem of the proposed solution was that it > didn't take into account the way the host config-space is accessed on the > DW PCIe. Depending on the DW PCIe IP-core synthesize parameters different > interfaces can be used to access the host and peripheral config/memory > spaces. The former one can be accessed via the DBI interface, while the > later ones is reached via the AHB/AXI application bus. In case if the DW > PCIe controller is configured to have a dedicated DBI interface, the way > it is mapped into the IO-memory turns to be platform-specific. For such > setups the DWC PCIe driver provides a set of the callbacks > dw_pcie_ops.{read_dbi,write_dbi} so the platforms glue-drivers would be > able to take into account the DBI bus IO peculiarities. Since > commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") these > methods haven't been utilized during the generic host initialization > performed by the PCIe subsystem code. > > I don't really know how come there have been no problems spotted for the > Histb/Exynos/Kirin PCIe controllers so far, but in our case with dword Because they implement their own pci_ops for the root bus. You should too. Who is 'our case'? > aligned IO requirement the generic config-space accessors can't be > utilized for the host config-space. Thus in order to make sure the host > config-space is properly accessed via the DBI bus let's get back the > dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() methods. They are going to > be just wrappers around the already defined > dw_pcie_read_dbi()/dw_pcie_write_dbi() functions with proper arguments > conversion. These methods perform the platform-specific config-space IO if > the DBI accessors are specified, otherwise they call normal MMIO > operations. The idea was for DWC to not define its own way to have different read/write for root bus vs. child bus as many PCI host bridges need the same thing. So the host bridge struct now has 2 pci_ops pointers. And the mess of function pointer indirection is gone. Rob
On Thu, May 26, 2022 at 04:29:30PM -0500, Rob Herring wrote: > On Tue, May 17, 2022 at 03:50:47PM +0300, Serge Semin wrote: > > Commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") replaced > > the locally defined DW PCIe host controller config-space accessors with > > the generic methods pci_generic_config_read() and > > pci_generic_config_write(). It was intended that the corresponding > > bus-mapping callback returned a correct virtual address of the passed PCI > > config-space register. The problem of the proposed solution was that it > > didn't take into account the way the host config-space is accessed on the > > DW PCIe. Depending on the DW PCIe IP-core synthesize parameters different > > interfaces can be used to access the host and peripheral config/memory > > spaces. The former one can be accessed via the DBI interface, while the > > later ones is reached via the AHB/AXI application bus. In case if the DW > > PCIe controller is configured to have a dedicated DBI interface, the way > > it is mapped into the IO-memory turns to be platform-specific. For such > > setups the DWC PCIe driver provides a set of the callbacks > > dw_pcie_ops.{read_dbi,write_dbi} so the platforms glue-drivers would be > > able to take into account the DBI bus IO peculiarities. Since > > commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") these > > methods haven't been utilized during the generic host initialization > > performed by the PCIe subsystem code. > > > > I don't really know how come there have been no problems spotted for the > > Histb/Exynos/Kirin PCIe controllers so far, but in our case with dword > > Because they implement their own pci_ops for the root bus. You should > too. Right. I should, but I would do that in a more generic way. Please see the next comment. > > Who is 'our case'? > > > aligned IO requirement the generic config-space accessors can't be > > utilized for the host config-space. Thus in order to make sure the host > > config-space is properly accessed via the DBI bus let's get back the > > dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() methods. They are going to > > be just wrappers around the already defined > > dw_pcie_read_dbi()/dw_pcie_write_dbi() functions with proper arguments > > conversion. These methods perform the platform-specific config-space IO if > > the DBI accessors are specified, otherwise they call normal MMIO > > operations. > > The idea was for DWC to not define its own way to have different > read/write for root bus vs. child bus as many PCI host bridges need the > same thing. So the host bridge struct now has 2 pci_ops pointers. And > the mess of function pointer indirection is gone. Thanks for clarification. I should have investigated the problem more thoroughly. Now I see what was the reason of that change. It was indeed wrong to blame the commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") that something was done incorrectly. After a more thorough commit inspection I realized that you just replaced the dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() with the generic pci_generic_config_read and pci_generic_config_write() as they had been equivalent anyway. I thought they didn't have the same semantic by confusing the dw_pcie_{read,write}() and dw_pcie_{read,write}_dbi() methods usage (see the _dbi suffix) in the original own PCI config-space accessors. So to speak I'll need to drop the Fixes tag with your commit hash from the patch. Getting back to the own-bus accessors. DW PCIe RP/EP own-config space is accessed over the DBI-bus. If the particular platform is designed in a way so the DBI MMIO space access has some non-specific peculiarities then that platform implements its own read_dbi/write_dbi accessors. In case if these callbacks are defined, the driver must use them for all DBI MMIO accesses including for the ones performed from the subsystem core in the framework of the host own config-space setups. As I mentioned in the patch log currently the only platforms with such requirement happen to be Histb, Exynos and Kirin DW PCIe. As such we can freely get back the generic dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() methods but use the dw_pcie_{read,write}_dbi() methods in there in the same way as it is done in the Histb, Exynos and Kirin DW PCIe drivers (see their own PCI config-space accessors match). Due to that we can drop the pci_ops redefinition from these platforms and just use the own-config space accessors for all such platforms as it's suggested in this patch. So this modification can be re-qualified to the cleanup one then: 1) Create the generic own config-space accessors (more portable as the DBI-bus access specifics must be always taken into account) as it is suggested in this patch already. 2) Drop the Kirin, Exynos, Histb own config-space re-definition. 3) Drop the dw_pcie_read_dbi() and dw_pcie_write_dbi() methods exporting. What do you think? -Sergey > > Rob
On Fri, May 27, 2022 at 07:05:55PM +0300, Serge Semin wrote: > On Thu, May 26, 2022 at 04:29:30PM -0500, Rob Herring wrote: > > On Tue, May 17, 2022 at 03:50:47PM +0300, Serge Semin wrote: > > > Commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") replaced > > > the locally defined DW PCIe host controller config-space accessors with > > > the generic methods pci_generic_config_read() and > > > pci_generic_config_write(). It was intended that the corresponding > > > bus-mapping callback returned a correct virtual address of the passed PCI > > > config-space register. The problem of the proposed solution was that it > > > didn't take into account the way the host config-space is accessed on the > > > DW PCIe. Depending on the DW PCIe IP-core synthesize parameters different > > > interfaces can be used to access the host and peripheral config/memory > > > spaces. The former one can be accessed via the DBI interface, while the > > > later ones is reached via the AHB/AXI application bus. In case if the DW > > > PCIe controller is configured to have a dedicated DBI interface, the way > > > it is mapped into the IO-memory turns to be platform-specific. For such > > > setups the DWC PCIe driver provides a set of the callbacks > > > dw_pcie_ops.{read_dbi,write_dbi} so the platforms glue-drivers would be > > > able to take into account the DBI bus IO peculiarities. Since > > > commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") these > > > methods haven't been utilized during the generic host initialization > > > performed by the PCIe subsystem code. > > > > > > I don't really know how come there have been no problems spotted for the > > > Histb/Exynos/Kirin PCIe controllers so far, but in our case with dword > > > > > Because they implement their own pci_ops for the root bus. You should > > too. > > Right. I should, but I would do that in a more generic way. Please see > the next comment. > > > > > Who is 'our case'? > > > > > aligned IO requirement the generic config-space accessors can't be > > > utilized for the host config-space. Thus in order to make sure the host > > > config-space is properly accessed via the DBI bus let's get back the > > > dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() methods. They are going to > > > be just wrappers around the already defined > > > dw_pcie_read_dbi()/dw_pcie_write_dbi() functions with proper arguments > > > conversion. These methods perform the platform-specific config-space IO if > > > the DBI accessors are specified, otherwise they call normal MMIO > > > operations. > > > > > The idea was for DWC to not define its own way to have different > > read/write for root bus vs. child bus as many PCI host bridges need the > > same thing. So the host bridge struct now has 2 pci_ops pointers. And > > the mess of function pointer indirection is gone. > > Thanks for clarification. I should have investigated the problem more > thoroughly. Now I see what was the reason of that change. It was > indeed wrong to blame the commit c2b0c098fbd1 ("PCI: dwc: Use generic > config accessors") that something was done incorrectly. After a more > thorough commit inspection I realized that you just replaced the > dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() with the generic > pci_generic_config_read and pci_generic_config_write() as they had > been equivalent anyway. I thought they didn't have the same semantic > by confusing the dw_pcie_{read,write}() and dw_pcie_{read,write}_dbi() > methods usage (see the _dbi suffix) in the original own PCI > config-space accessors. So to speak I'll need to drop the Fixes tag > with your commit hash from the patch. > > Getting back to the own-bus accessors. DW PCIe RP/EP own-config space > is accessed over the DBI-bus. If the particular platform is designed > in a way so the DBI MMIO space access has some non-specific > peculiarities then that platform implements its own read_dbi/write_dbi > accessors. In case if these callbacks are defined, the driver must > use them for all DBI MMIO accesses including for the ones performed > from the subsystem core in the framework of the host own config-space > setups. As I mentioned in the patch log currently the only platforms > with such requirement happen to be Histb, Exynos and Kirin DW PCIe. As > such we can freely get back the generic dw_pcie_rd_own_conf() and > dw_pcie_wr_own_conf() methods but use the dw_pcie_{read,write}_dbi() > methods in there in the same way as it is done in the Histb, Exynos > and Kirin DW PCIe drivers (see their own PCI config-space accessors > match). Due to that we can drop the pci_ops redefinition from these > platforms and just use the own-config space accessors for all such > platforms as it's suggested in this patch. So this modification can be > re-qualified to the cleanup one then: > 1) Create the generic own config-space accessors (more portable as > the DBI-bus access specifics must be always taken into account) as it > is suggested in this patch already. > 2) Drop the Kirin, Exynos, Histb own config-space re-definition. > 3) Drop the dw_pcie_read_dbi() and dw_pcie_write_dbi() methods exporting. Alas this can't be implemented. I forgot about the inliners defined in the pcie-designware.h file. But the rest of the denoted above cleanups still can be (Kirin under question though). -Sergey > > What do you think? > > -Sergey > > > > > Rob
On Fri, May 27, 2022 at 08:39:53PM +0300, Serge Semin wrote: > On Fri, May 27, 2022 at 07:05:55PM +0300, Serge Semin wrote: > > On Thu, May 26, 2022 at 04:29:30PM -0500, Rob Herring wrote: > > > On Tue, May 17, 2022 at 03:50:47PM +0300, Serge Semin wrote: > > > > Commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") replaced > > > > the locally defined DW PCIe host controller config-space accessors with > > > > the generic methods pci_generic_config_read() and > > > > pci_generic_config_write(). It was intended that the corresponding > > > > bus-mapping callback returned a correct virtual address of the passed PCI > > > > config-space register. The problem of the proposed solution was that it > > > > didn't take into account the way the host config-space is accessed on the > > > > DW PCIe. Depending on the DW PCIe IP-core synthesize parameters different > > > > interfaces can be used to access the host and peripheral config/memory > > > > spaces. The former one can be accessed via the DBI interface, while the > > > > later ones is reached via the AHB/AXI application bus. In case if the DW > > > > PCIe controller is configured to have a dedicated DBI interface, the way > > > > it is mapped into the IO-memory turns to be platform-specific. For such > > > > setups the DWC PCIe driver provides a set of the callbacks > > > > dw_pcie_ops.{read_dbi,write_dbi} so the platforms glue-drivers would be > > > > able to take into account the DBI bus IO peculiarities. Since > > > > commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") these > > > > methods haven't been utilized during the generic host initialization > > > > performed by the PCIe subsystem code. > > > > > > > > I don't really know how come there have been no problems spotted for the > > > > Histb/Exynos/Kirin PCIe controllers so far, but in our case with dword > > > > > > > > Because they implement their own pci_ops for the root bus. You should > > > too. > > > > Right. I should, but I would do that in a more generic way. Please see > > the next comment. > > > > > > > > Who is 'our case'? > > > > > > > aligned IO requirement the generic config-space accessors can't be > > > > utilized for the host config-space. Thus in order to make sure the host > > > > config-space is properly accessed via the DBI bus let's get back the > > > > dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() methods. They are going to > > > > be just wrappers around the already defined > > > > dw_pcie_read_dbi()/dw_pcie_write_dbi() functions with proper arguments > > > > conversion. These methods perform the platform-specific config-space IO if > > > > the DBI accessors are specified, otherwise they call normal MMIO > > > > operations. > > > > > > > > The idea was for DWC to not define its own way to have different > > > read/write for root bus vs. child bus as many PCI host bridges need the > > > same thing. So the host bridge struct now has 2 pci_ops pointers. And > > > the mess of function pointer indirection is gone. > > > > Thanks for clarification. I should have investigated the problem more > > thoroughly. Now I see what was the reason of that change. It was > > indeed wrong to blame the commit c2b0c098fbd1 ("PCI: dwc: Use generic > > config accessors") that something was done incorrectly. After a more > > thorough commit inspection I realized that you just replaced the > > dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() with the generic > > pci_generic_config_read and pci_generic_config_write() as they had > > been equivalent anyway. I thought they didn't have the same semantic > > by confusing the dw_pcie_{read,write}() and dw_pcie_{read,write}_dbi() > > methods usage (see the _dbi suffix) in the original own PCI > > config-space accessors. So to speak I'll need to drop the Fixes tag > > with your commit hash from the patch. > > > > Getting back to the own-bus accessors. DW PCIe RP/EP own-config space > > is accessed over the DBI-bus. If the particular platform is designed > > in a way so the DBI MMIO space access has some non-specific > > peculiarities then that platform implements its own read_dbi/write_dbi > > accessors. In case if these callbacks are defined, the driver must > > use them for all DBI MMIO accesses including for the ones performed > > from the subsystem core in the framework of the host own config-space > > setups. As I mentioned in the patch log currently the only platforms > > with such requirement happen to be Histb, Exynos and Kirin DW PCIe. As > > such we can freely get back the generic dw_pcie_rd_own_conf() and > > dw_pcie_wr_own_conf() methods but use the dw_pcie_{read,write}_dbi() > > methods in there in the same way as it is done in the Histb, Exynos > > and Kirin DW PCIe drivers (see their own PCI config-space accessors > > match). Due to that we can drop the pci_ops redefinition from these > > platforms and just use the own-config space accessors for all such > > platforms as it's suggested in this patch. So this modification can be > > re-qualified to the cleanup one then: > > 1) Create the generic own config-space accessors (more portable as > > the DBI-bus access specifics must be always taken into account) as it > > is suggested in this patch already. That is the wrong direction IMO. The idea is that well behaved cases just use the generic code and avoid any driver specific code. The DWC common code is not generic code. It's also keeping with the "don't create mid layers" philosophy. We have generic 32-bit only accessors too (even though that's broken h/w, it's broken so often we needed generic accessors), so if that's your restriction, then use those. That way, it is very clear which drivers (all of them, not just DWC) use generic accessors, have alignment restrictions, or something completely custom. > > 2) Drop the Kirin, Exynos, Histb own config-space re-definition. Those drivers are special. They get to keep their special code. > > 3) Drop the dw_pcie_read_dbi() and dw_pcie_write_dbi() methods exporting. > > Alas this can't be implemented. I forgot about the inliners defined in the > pcie-designware.h file. But the rest of the denoted above cleanups still > can be (Kirin under question though).
On Tue, May 31, 2022 at 11:09:07AM -0500, Rob Herring wrote: > On Fri, May 27, 2022 at 08:39:53PM +0300, Serge Semin wrote: > > On Fri, May 27, 2022 at 07:05:55PM +0300, Serge Semin wrote: > > > On Thu, May 26, 2022 at 04:29:30PM -0500, Rob Herring wrote: > > > > On Tue, May 17, 2022 at 03:50:47PM +0300, Serge Semin wrote: > > > > > Commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") replaced > > > > > the locally defined DW PCIe host controller config-space accessors with > > > > > the generic methods pci_generic_config_read() and > > > > > pci_generic_config_write(). It was intended that the corresponding > > > > > bus-mapping callback returned a correct virtual address of the passed PCI > > > > > config-space register. The problem of the proposed solution was that it > > > > > didn't take into account the way the host config-space is accessed on the > > > > > DW PCIe. Depending on the DW PCIe IP-core synthesize parameters different > > > > > interfaces can be used to access the host and peripheral config/memory > > > > > spaces. The former one can be accessed via the DBI interface, while the > > > > > later ones is reached via the AHB/AXI application bus. In case if the DW > > > > > PCIe controller is configured to have a dedicated DBI interface, the way > > > > > it is mapped into the IO-memory turns to be platform-specific. For such > > > > > setups the DWC PCIe driver provides a set of the callbacks > > > > > dw_pcie_ops.{read_dbi,write_dbi} so the platforms glue-drivers would be > > > > > able to take into account the DBI bus IO peculiarities. Since > > > > > commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") these > > > > > methods haven't been utilized during the generic host initialization > > > > > performed by the PCIe subsystem code. > > > > > > > > > > I don't really know how come there have been no problems spotted for the > > > > > Histb/Exynos/Kirin PCIe controllers so far, but in our case with dword > > > > > > > > > > > Because they implement their own pci_ops for the root bus. You should > > > > too. > > > > > > Right. I should, but I would do that in a more generic way. Please see > > > the next comment. > > > > > > > > > > > Who is 'our case'? > > > > > > > > > aligned IO requirement the generic config-space accessors can't be > > > > > utilized for the host config-space. Thus in order to make sure the host > > > > > config-space is properly accessed via the DBI bus let's get back the > > > > > dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() methods. They are going to > > > > > be just wrappers around the already defined > > > > > dw_pcie_read_dbi()/dw_pcie_write_dbi() functions with proper arguments > > > > > conversion. These methods perform the platform-specific config-space IO if > > > > > the DBI accessors are specified, otherwise they call normal MMIO > > > > > operations. > > > > > > > > > > > The idea was for DWC to not define its own way to have different > > > > read/write for root bus vs. child bus as many PCI host bridges need the > > > > same thing. So the host bridge struct now has 2 pci_ops pointers. And > > > > the mess of function pointer indirection is gone. > > > > > > Thanks for clarification. I should have investigated the problem more > > > thoroughly. Now I see what was the reason of that change. It was > > > indeed wrong to blame the commit c2b0c098fbd1 ("PCI: dwc: Use generic > > > config accessors") that something was done incorrectly. After a more > > > thorough commit inspection I realized that you just replaced the > > > dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() with the generic > > > pci_generic_config_read and pci_generic_config_write() as they had > > > been equivalent anyway. I thought they didn't have the same semantic > > > by confusing the dw_pcie_{read,write}() and dw_pcie_{read,write}_dbi() > > > methods usage (see the _dbi suffix) in the original own PCI > > > config-space accessors. So to speak I'll need to drop the Fixes tag > > > with your commit hash from the patch. > > > > > > Getting back to the own-bus accessors. DW PCIe RP/EP own-config space > > > is accessed over the DBI-bus. If the particular platform is designed > > > in a way so the DBI MMIO space access has some non-specific > > > peculiarities then that platform implements its own read_dbi/write_dbi > > > accessors. In case if these callbacks are defined, the driver must > > > use them for all DBI MMIO accesses including for the ones performed > > > from the subsystem core in the framework of the host own config-space > > > setups. As I mentioned in the patch log currently the only platforms > > > with such requirement happen to be Histb, Exynos and Kirin DW PCIe. As > > > such we can freely get back the generic dw_pcie_rd_own_conf() and > > > dw_pcie_wr_own_conf() methods but use the dw_pcie_{read,write}_dbi() > > > methods in there in the same way as it is done in the Histb, Exynos > > > and Kirin DW PCIe drivers (see their own PCI config-space accessors > > > match). Due to that we can drop the pci_ops redefinition from these > > > platforms and just use the own-config space accessors for all such > > > platforms as it's suggested in this patch. So this modification can be > > > re-qualified to the cleanup one then: > > > 1) Create the generic own config-space accessors (more portable as > > > the DBI-bus access specifics must be always taken into account) as it > > > is suggested in this patch already. > > That is the wrong direction IMO. The idea is that well behaved cases > just use the generic code and avoid any driver specific code. The DWC > common code is not generic code. It's also keeping with the "don't > create mid layers" philosophy. Got it. Thanks for clarification. So far I has been sure that re-using the locally implemented specifics was more preferable. It was so obvious for me that I missed there can be the PCI common code requirements. Though it would be nice to have it described somewhere in the kernel docs. > > We have generic 32-bit only accessors too (even though that's broken > h/w, it's broken so often we needed generic accessors), so if that's > your restriction, then use those. That way, it is very clear which > drivers (all of them, not just DWC) use generic accessors, have > alignment restrictions, or something completely custom. Oh, I didn't know about them. Thanks for pointing out on those methods. I'll use them in my driver then. > > > > 2) Drop the Kirin, Exynos, Histb own config-space re-definition. > > Those drivers are special. They get to keep their special code. It seems to me my driver will be another special case. But instead of re-implementing the pci_ops.{read,write} accessors it will use the dword-aligned generic config read/write functions. -Sergey > > > > 3) Drop the dw_pcie_read_dbi() and dw_pcie_write_dbi() methods exporting. > > > > Alas this can't be implemented. I forgot about the inliners defined in the > > pcie-designware.h file. But the rest of the denoted above cleanups still > > can be (Kirin under question though).
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 7403b1709726..a250869334a5 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -534,10 +534,40 @@ void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, unsigned int devfn, } EXPORT_SYMBOL_GPL(dw_pcie_own_conf_map_bus); +static int dw_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 *val) +{ + struct pcie_port *pp = bus->sysdata; + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); + + if (PCI_SLOT(devfn) > 0) { + *val = ~0U; + return PCIBIOS_DEVICE_NOT_FOUND; + } + + *val = dw_pcie_read_dbi(pci, where, size); + + return PCIBIOS_SUCCESSFUL; +} + +static int dw_pcie_wr_own_conf(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 val) +{ + struct pcie_port *pp = bus->sysdata; + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); + + if (PCI_SLOT(devfn) > 0) + return PCIBIOS_DEVICE_NOT_FOUND; + + dw_pcie_write_dbi(pci, where, size, val); + + return PCIBIOS_SUCCESSFUL; +} + static struct pci_ops dw_pcie_ops = { .map_bus = dw_pcie_own_conf_map_bus, - .read = pci_generic_config_read, - .write = pci_generic_config_write, + .read = dw_pcie_rd_own_conf, + .write = dw_pcie_wr_own_conf, }; void dw_pcie_setup_rc(struct pcie_port *pp)
Commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") replaced the locally defined DW PCIe host controller config-space accessors with the generic methods pci_generic_config_read() and pci_generic_config_write(). It was intended that the corresponding bus-mapping callback returned a correct virtual address of the passed PCI config-space register. The problem of the proposed solution was that it didn't take into account the way the host config-space is accessed on the DW PCIe. Depending on the DW PCIe IP-core synthesize parameters different interfaces can be used to access the host and peripheral config/memory spaces. The former one can be accessed via the DBI interface, while the later ones is reached via the AHB/AXI application bus. In case if the DW PCIe controller is configured to have a dedicated DBI interface, the way it is mapped into the IO-memory turns to be platform-specific. For such setups the DWC PCIe driver provides a set of the callbacks dw_pcie_ops.{read_dbi,write_dbi} so the platforms glue-drivers would be able to take into account the DBI bus IO peculiarities. Since commit c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") these methods haven't been utilized during the generic host initialization performed by the PCIe subsystem code. I don't really know how come there have been no problems spotted for the Histb/Exynos/Kirin PCIe controllers so far, but in our case with dword aligned IO requirement the generic config-space accessors can't be utilized for the host config-space. Thus in order to make sure the host config-space is properly accessed via the DBI bus let's get back the dw_pcie_rd_own_conf() and dw_pcie_wr_own_conf() methods. They are going to be just wrappers around the already defined dw_pcie_read_dbi()/dw_pcie_write_dbi() functions with proper arguments conversion. These methods perform the platform-specific config-space IO if the DBI accessors are specified, otherwise they call normal MMIO operations. Fixes: c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- .../pci/controller/dwc/pcie-designware-host.c | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)