| Message ID | 20200821035420.380495-1-robh@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | PCI: dwc: Driver clean-ups | expand |
On Thu, Aug 20, 2020 at 09:53:40PM -0600, Rob Herring wrote: > This is a series of clean-ups for the Designware PCI driver. The series > initially reworks the config space accessors to use the existing pci_ops > struct. Then there's removal of various private data that's also present > in the pci_host_bridge struct. There's also some duplicated common (PCI > and DWC) register defines which I converted to use the common defines. > Finally, the initialization for speed/gen, number of lanes, and N_FTS > are all moved to the common DWC code. > > This is compile tested only as I don't have any DWC based h/w, so any > testing would be helpful. A branch is here[1]. Applied the series to pci/dwc, thanks. Lorenzo > Rob > > [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git pci-dw-cleanups > > Rob Herring (40): > PCI: Allow root and child buses to have different pci_ops > PCI: dwc: Use DBI accessors instead of own config accessors > PCI: dwc: Allow overriding bridge pci_ops > PCI: dwc: Add a default pci_ops.map_bus for root port > PCI: dwc: al: Use pci_ops for child config space accessors > PCI: dwc: keystone: Use pci_ops for config space accessors > PCI: dwc: tegra: Use pci_ops for root config space accessors > PCI: dwc: meson: Use pci_ops for root config space accessors > PCI: dwc: kirin: Use pci_ops for root config space accessors > PCI: dwc: exynos: Use pci_ops for root config space accessors > PCI: dwc: histb: Use pci_ops for root config space accessors > PCI: dwc: Remove dwc specific config accessor ops > PCI: dwc: Use generic config accessors > PCI: Also call .add_bus() callback for root bus > PCI: dwc: keystone: Convert .scan_bus() callback to use add_bus > PCI: dwc: Convert to use pci_host_probe() > PCI: dwc: Remove root_bus pointer > PCI: dwc: Remove storing of PCI resources > PCI: dwc: Simplify config space handling > PCI: dwc/keystone: Drop duplicated 'num-viewport' > PCI: dwc: Check CONFIG_PCI_MSI inside dw_pcie_msi_init() > PCI: dwc/imx6: Remove duplicate define PCIE_LINK_WIDTH_SPEED_CONTROL > PCI: dwc: Add a 'num_lanes' field to struct dw_pcie > PCI: dwc: Ensure FAST_LINK_MODE is cleared > PCI: dwc/meson: Drop the duplicate number of lanes setup > PCI: dwc/meson: Drop unnecessary RC config space initialization > PCI: dwc/meson: Rework PCI config and DW port logic register accesses > PCI: dwc/imx6: Use common PCI register definitions > PCI: dwc/qcom: Use common PCI register definitions > PCI: dwc: Remove hardcoded PCI_CAP_ID_EXP offset > PCI: dwc/tegra: Use common Designware port logic register definitions > PCI: dwc: Remove read_dbi2 code > PCI: dwc: Make ATU accessors private > PCI: dwc: Centralize link gen setting > PCI: dwc: Set PORT_LINK_DLL_LINK_EN in common setup code > PCI: dwc/intel-gw: Drop unnecessary checking of DT 'device_type' > property > PCI: dwc/intel-gw: Move getting PCI_CAP_ID_EXP offset to > intel_pcie_link_setup() > PCI: dwc/intel-gw: Drop unused max_width > PCI: dwc: Move N_FTS setup to common setup > PCI: dwc: Use DBI accessors > > drivers/pci/controller/dwc/pci-dra7xx.c | 29 +- > drivers/pci/controller/dwc/pci-exynos.c | 45 +-- > drivers/pci/controller/dwc/pci-imx6.c | 52 +-- > drivers/pci/controller/dwc/pci-keystone.c | 126 ++----- > drivers/pci/controller/dwc/pci-meson.c | 156 ++------- > drivers/pci/controller/dwc/pcie-al.c | 70 +--- > drivers/pci/controller/dwc/pcie-artpec6.c | 48 +-- > .../pci/controller/dwc/pcie-designware-ep.c | 11 +- > .../pci/controller/dwc/pcie-designware-host.c | 319 ++++++------------ > .../pci/controller/dwc/pcie-designware-plat.c | 4 +- > drivers/pci/controller/dwc/pcie-designware.c | 104 +++--- > drivers/pci/controller/dwc/pcie-designware.h | 54 +-- > drivers/pci/controller/dwc/pcie-histb.c | 45 +-- > drivers/pci/controller/dwc/pcie-intel-gw.c | 65 +--- > drivers/pci/controller/dwc/pcie-kirin.c | 43 +-- > drivers/pci/controller/dwc/pcie-qcom.c | 33 +- > drivers/pci/controller/dwc/pcie-spear13xx.c | 39 +-- > drivers/pci/controller/dwc/pcie-tegra194.c | 120 ++----- > drivers/pci/controller/dwc/pcie-uniphier.c | 3 +- > drivers/pci/probe.c | 14 +- > include/linux/pci.h | 1 + > 21 files changed, 443 insertions(+), 938 deletions(-) > > -- > 2.25.1
Hi Rob, > This is a series of clean-ups for the Designware PCI driver. The series > initially reworks the config space accessors to use the existing pci_ops > struct. Then there's removal of various private data that's also present > in the pci_host_bridge struct. There's also some duplicated common (PCI > and DWC) register defines which I converted to use the common defines. > Finally, the initialization for speed/gen, number of lanes, and N_FTS > are all moved to the common DWC code. > This is compile tested only as I don't have any DWC based h/w, so any > testing would be helpful. A branch is here[1]. I've noticed that with the latest linux-next, my board doesn't boot anymore. I've traced it back to this series. There is a similar board in kernelci [1,2] where you can have a look at the backtrace. I've bisected this to the following patch: PCI: dwc: Use generic config accessors I'm pretty much lost here. It seems that the kernel tries to read from an invalid/unmapped memory address. [1] https://kernelci.org/test/plan/id/5f5f4992d1c53777a0a6092d/ [2] https://storage.kernelci.org/next/master/next-20200914/arm64/defconfig/gcc-8/lab-nxp/baseline-fsl-ls1028a-rdb.txt -michael
On Tue, Sep 15, 2020 at 3:12 AM Michael Walle <michael@walle.cc> wrote: > > Hi Rob, > > > This is a series of clean-ups for the Designware PCI driver. The series > > initially reworks the config space accessors to use the existing pci_ops > > struct. Then there's removal of various private data that's also present > > in the pci_host_bridge struct. There's also some duplicated common (PCI > > and DWC) register defines which I converted to use the common defines. > > Finally, the initialization for speed/gen, number of lanes, and N_FTS > > are all moved to the common DWC code. > > > This is compile tested only as I don't have any DWC based h/w, so any > > testing would be helpful. A branch is here[1]. > > I've noticed that with the latest linux-next, my board doesn't boot > anymore. I've traced it back to this series. There is a similar > board in kernelci [1,2] where you can have a look at the backtrace. > > I've bisected this to the following patch: > PCI: dwc: Use generic config accessors That's helpful. > I'm pretty much lost here. It seems that the kernel tries to read from > an invalid/unmapped memory address. > > [1] https://kernelci.org/test/plan/id/5f5f4992d1c53777a0a6092d/ > [2] https://storage.kernelci.org/next/master/next-20200914/arm64/defconfig/gcc-8/lab-nxp/baseline-fsl-ls1028a-rdb.txt Thanks for the pointers. I was wondering if kernelci had any boards with DWC. Can you try this? The link up check seemed unnecessary as it is racy. What happens if the link goes down right after checking? That's the only thing in the change that sticks out. diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 317ff512f8df..afee1a0e8883 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -441,6 +441,9 @@ static void __iomem *dw_pcie_other_conf_map_bus(struct pci_bus *bus, struct pcie_port *pp = bus->sysdata; struct dw_pcie *pci = to_dw_pcie_from_pp(pp); + if (!dw_pcie_link_up(pci)) + return NULL; + busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) | PCIE_ATU_FUNC(PCI_FUNC(devfn));
Am 2020-09-16 00:02, schrieb Rob Herring: > Can you try this? The link up check seemed unnecessary as it is racy. > What happens if the link goes down right after checking? That's the > only thing in the change that sticks out. > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c > b/drivers/pci/controller/dwc/pcie-designware-host.c > index 317ff512f8df..afee1a0e8883 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -441,6 +441,9 @@ static void __iomem > *dw_pcie_other_conf_map_bus(struct pci_bus *bus, > struct pcie_port *pp = bus->sysdata; > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > + if (!dw_pcie_link_up(pci)) > + return NULL; > + > busdev = PCIE_ATU_BUS(bus->number) | > PCIE_ATU_DEV(PCI_SLOT(devfn)) | > PCIE_ATU_FUNC(PCI_FUNC(devfn)); This will fix the issue. -michael
Hi, On 16/09/20 1:24 pm, Michael Walle wrote: > Am 2020-09-16 00:02, schrieb Rob Herring: >> Can you try this? The link up check seemed unnecessary as it is racy. >> What happens if the link goes down right after checking? That's the >> only thing in the change that sticks out. >> >> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c >> b/drivers/pci/controller/dwc/pcie-designware-host.c >> index 317ff512f8df..afee1a0e8883 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware-host.c >> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c >> @@ -441,6 +441,9 @@ static void __iomem >> *dw_pcie_other_conf_map_bus(struct pci_bus *bus, >> struct pcie_port *pp = bus->sysdata; >> struct dw_pcie *pci = to_dw_pcie_from_pp(pp); >> >> + if (!dw_pcie_link_up(pci)) >> + return NULL; >> + >> busdev = PCIE_ATU_BUS(bus->number) | >> PCIE_ATU_DEV(PCI_SLOT(devfn)) | >> PCIE_ATU_FUNC(PCI_FUNC(devfn)); > > This will fix the issue. This fix is required to get DRA7 EVM booting again in linux-next. Thanks Kishon
On Tue, Sep 29, 2020 at 12:24 AM Kishon Vijay Abraham I <kishon@ti.com> wrote: > > Hi, > > On 16/09/20 1:24 pm, Michael Walle wrote: > > Am 2020-09-16 00:02, schrieb Rob Herring: > >> Can you try this? The link up check seemed unnecessary as it is racy. > >> What happens if the link goes down right after checking? That's the > >> only thing in the change that sticks out. > >> > >> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c > >> b/drivers/pci/controller/dwc/pcie-designware-host.c > >> index 317ff512f8df..afee1a0e8883 100644 > >> --- a/drivers/pci/controller/dwc/pcie-designware-host.c > >> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > >> @@ -441,6 +441,9 @@ static void __iomem > >> *dw_pcie_other_conf_map_bus(struct pci_bus *bus, > >> struct pcie_port *pp = bus->sysdata; > >> struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > >> > >> + if (!dw_pcie_link_up(pci)) > >> + return NULL; > >> + > >> busdev = PCIE_ATU_BUS(bus->number) | > >> PCIE_ATU_DEV(PCI_SLOT(devfn)) | > >> PCIE_ATU_FUNC(PCI_FUNC(devfn)); > > > > This will fix the issue. > > This fix is required to get DRA7 EVM booting again in linux-next. Did you see the discussion here[1]? Is firmware setting up the same register in question? Rob [1] http://lore.kernel.org/r/HE1PR0402MB33713A623A37D08AE3253DEB84320@HE1PR0402MB3371.eurprd04.prod.outlook.com
This is a series of clean-ups for the Designware PCI driver. The series initially reworks the config space accessors to use the existing pci_ops struct. Then there's removal of various private data that's also present in the pci_host_bridge struct. There's also some duplicated common (PCI and DWC) register defines which I converted to use the common defines. Finally, the initialization for speed/gen, number of lanes, and N_FTS are all moved to the common DWC code. This is compile tested only as I don't have any DWC based h/w, so any testing would be helpful. A branch is here[1]. Rob [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git pci-dw-cleanups Rob Herring (40): PCI: Allow root and child buses to have different pci_ops PCI: dwc: Use DBI accessors instead of own config accessors PCI: dwc: Allow overriding bridge pci_ops PCI: dwc: Add a default pci_ops.map_bus for root port PCI: dwc: al: Use pci_ops for child config space accessors PCI: dwc: keystone: Use pci_ops for config space accessors PCI: dwc: tegra: Use pci_ops for root config space accessors PCI: dwc: meson: Use pci_ops for root config space accessors PCI: dwc: kirin: Use pci_ops for root config space accessors PCI: dwc: exynos: Use pci_ops for root config space accessors PCI: dwc: histb: Use pci_ops for root config space accessors PCI: dwc: Remove dwc specific config accessor ops PCI: dwc: Use generic config accessors PCI: Also call .add_bus() callback for root bus PCI: dwc: keystone: Convert .scan_bus() callback to use add_bus PCI: dwc: Convert to use pci_host_probe() PCI: dwc: Remove root_bus pointer PCI: dwc: Remove storing of PCI resources PCI: dwc: Simplify config space handling PCI: dwc/keystone: Drop duplicated 'num-viewport' PCI: dwc: Check CONFIG_PCI_MSI inside dw_pcie_msi_init() PCI: dwc/imx6: Remove duplicate define PCIE_LINK_WIDTH_SPEED_CONTROL PCI: dwc: Add a 'num_lanes' field to struct dw_pcie PCI: dwc: Ensure FAST_LINK_MODE is cleared PCI: dwc/meson: Drop the duplicate number of lanes setup PCI: dwc/meson: Drop unnecessary RC config space initialization PCI: dwc/meson: Rework PCI config and DW port logic register accesses PCI: dwc/imx6: Use common PCI register definitions PCI: dwc/qcom: Use common PCI register definitions PCI: dwc: Remove hardcoded PCI_CAP_ID_EXP offset PCI: dwc/tegra: Use common Designware port logic register definitions PCI: dwc: Remove read_dbi2 code PCI: dwc: Make ATU accessors private PCI: dwc: Centralize link gen setting PCI: dwc: Set PORT_LINK_DLL_LINK_EN in common setup code PCI: dwc/intel-gw: Drop unnecessary checking of DT 'device_type' property PCI: dwc/intel-gw: Move getting PCI_CAP_ID_EXP offset to intel_pcie_link_setup() PCI: dwc/intel-gw: Drop unused max_width PCI: dwc: Move N_FTS setup to common setup PCI: dwc: Use DBI accessors drivers/pci/controller/dwc/pci-dra7xx.c | 29 +- drivers/pci/controller/dwc/pci-exynos.c | 45 +-- drivers/pci/controller/dwc/pci-imx6.c | 52 +-- drivers/pci/controller/dwc/pci-keystone.c | 126 ++----- drivers/pci/controller/dwc/pci-meson.c | 156 ++------- drivers/pci/controller/dwc/pcie-al.c | 70 +--- drivers/pci/controller/dwc/pcie-artpec6.c | 48 +-- .../pci/controller/dwc/pcie-designware-ep.c | 11 +- .../pci/controller/dwc/pcie-designware-host.c | 319 ++++++------------ .../pci/controller/dwc/pcie-designware-plat.c | 4 +- drivers/pci/controller/dwc/pcie-designware.c | 104 +++--- drivers/pci/controller/dwc/pcie-designware.h | 54 +-- drivers/pci/controller/dwc/pcie-histb.c | 45 +-- drivers/pci/controller/dwc/pcie-intel-gw.c | 65 +--- drivers/pci/controller/dwc/pcie-kirin.c | 43 +-- drivers/pci/controller/dwc/pcie-qcom.c | 33 +- drivers/pci/controller/dwc/pcie-spear13xx.c | 39 +-- drivers/pci/controller/dwc/pcie-tegra194.c | 120 ++----- drivers/pci/controller/dwc/pcie-uniphier.c | 3 +- drivers/pci/probe.c | 14 +- include/linux/pci.h | 1 + 21 files changed, 443 insertions(+), 938 deletions(-) -- 2.25.1