PCI: dwc: Revert Link Up IRQ support
Subsystems (7)
v2 cover PCI: dwc: Revert Link Up IRQ support
Revert all patches related to pcie-designware Root Complex Link Up IRQ support.
While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
Changes since v1:
-Rebased against latest pci/controller/dwc
-Picked up tags.
Niklas Cassel (6):
Revert "PCI: dw-rockchip: Don't wait for link since we can detect Link
Up"
Revert "PCI: dw-rockchip: Enumerate endpoints based on dll_link_up
IRQ"
Revert "PCI: qcom: Don't wait for link if we can detect Link Up" Revert "PCI: qcom: Enable MSI interrupts together with Link up if
'Global IRQ' is supported"
Revert "PCI: qcom: Enumerate endpoints based on Link up event in
'global_irq' interrupt"
Revert "PCI: dwc: Don't wait for link up if driver can detect Link Up
event"
.../pci/controller/dwc/pcie-designware-host.c | 10 +-- drivers/pci/controller/dwc/pcie-designware.h | 1 - drivers/pci/controller/dwc/pcie-dw-rockchip.c | 60 +----------------- drivers/pci/controller/dwc/pcie-qcom.c | 63 +------------------ 4 files changed, 6 insertions(+), 128 deletions(-)
base-commit: cfd2fdfd0a8da2e5bbfdc4009b9c4b8bf164c937
Signature · 1 line
2.52.0
PATCH v2 1/6 Revert "PCI: dw-rockchip: Don't wait for link since we can detect Link Up"
This reverts commit ec9fd499b9c60a187ac8d6414c3c343c77d32e42.
While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 1 - 1 file changed, 1 deletion(-)
drivers/pci/controller/dwc/pcie-dw-rockchip.c +0 -1
@@ -601,7 +601,6 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev, pp = &rockchip->pci.pp; pp->ops = &rockchip_pcie_host_ops; - pp->use_linkup_irq = true; ret =dw_pcie_host_init (pp);if (ret) {
Signature · 1 line
2.52.0
Re: [PATCH v2 1/6] Revert "PCI: dw-rockchip: Don't wait for link since we can detect Link Up"
[+cc Ilpo]
Quoted 5 lines · Niklas Cassel
This reverts commit
ec9fd499b9c60a187ac8d6414c3c343c77d32e42.While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus0004:43:busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
Apart from the actual problem you're fixing, it looks like some of our bus number resource setup is a little deficient here. The "43-41" and "42-41" ranges are bogus and "(null)" doesn't tell us anything useful.
Quoted 26 lines
pci_bus0004:43:busn_res: [bus 43-41] end is updated to 43pci_bus0004:43:busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for thempci_bus0004:44:busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:44:busn_res: [bus 44-41] end is updated to 44pci_bus0004:44:busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for thempci_bus0004:45:busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:45:busn_res: [bus 45-41] end is updated to 45pci_bus0004:45:busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for thempci_bus0004:46:busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:46:busn_res: [bus 46-41] end is updated to 46pci_bus0004:46:busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for thempci_bus0004:42:busn_res: [bus 42-41] end is updated to 46pci_bus0004:42:busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
Quoted 5 lines
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
I suppose this is could also be viewed as a resource management problem.
Even in the normal hotplug case, we never know what a hot-added hierarchy will look like, so we hack around it with the compiled-in DEFAULT_HOTPLUG_BUS_SIZE and the "hpbussize=" kernel parameter.
It would be nice if we could allocate more bus numbers at hot-add (or link-up) time as needed. That's hard after drivers have claimed nearby devices, but in this case there might not be anything nearby.
Quoted 2 lines
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
"Proper" is hiding some important details here. pwrctrl already supports powering up and enumerating switches. I think this refers to [1], where Mani says the plan is for pwrctrl to power up everything before the initial bus scan so we can figure out how many buses are needed.
Bjorn
[1] https://lore.kernel.org/linux-pci/fle74skju2rorxmfdvosmeyrx3g75rysuszov5ofvde2exj4ir@3kfjyfyhczmn/
Re: [PATCH v2 1/6] Revert "PCI: dw-rockchip: Don't wait for link since we can detect Link Up"
Hello Bjorn,
Quoted 10 lines · Bjorn Helgaas
Quoted 2 lines
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
"Proper" is hiding some important details here. pwrctrl already supports powering up and enumerating switches. I think this refers to [1], where Mani says the plan is for pwrctrl to power up everything before the initial bus scan so we can figure out how many buses are needed.
[1] https://lore.kernel.org/linux-pci/fle74skju2rorxmfdvosmeyrx3g75rysuszov5ofvde2exj4ir@3kfjyfyhczmn/
Correct.
This series has already been merged to the pci/controller/dwc branch (although for some reason this branch is not merged to pci/next yet), and I can see that Mani have rephrased the commit messages to be more specific and to avoid the word proper.
Please tell me if there is some futher action needed from my part.
Kind regards,
Niklas
PATCH v2 2/6 Revert "PCI: dw-rockchip: Enumerate endpoints based on dll_link_up IRQ"
In reply to Niklas CasselThis reverts commit 0e0b45ab5d770a748487ba0ae8f77d1fb0f0de3e.
While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 59 +------------------ 1 file changed, 3 insertions(+), 56 deletions(-)
drivers/pci/controller/dwc/pcie-dw-rockchip.c +3 -56
@@ -508,34 +508,6 @@ static const struct dw_pcie_ops dw_pcie_ops = { .get_ltssm = rockchip_pcie_get_ltssm, }; -static irqreturn_t rockchip_pcie_rc_sys_irq_thread (int irq,void *arg) -{ -struct rockchip_pcie *rockchip = arg; -struct dw_pcie *pci = &rockchip->pci; -struct dw_pcie_rp *pp = &pci->pp; -struct device *dev = pci->dev; -u32 reg; - - reg =rockchip_pcie_readl_apb (rockchip,PCIE_CLIENT_INTR_STATUS_MISC ); -rockchip_pcie_writel_apb (rockchip, reg,PCIE_CLIENT_INTR_STATUS_MISC ); - -dev_dbg (dev,"PCIE_CLIENT_INTR_STATUS_MISC: %#x\n" , reg); -dev_dbg (dev,"LTSSM_STATUS: %#x\n" ,rockchip_pcie_get_ltssm_reg (rockchip)); - -if (reg &PCIE_RDLH_LINK_UP_CHGED ) { -if (rockchip_pcie_link_up (pci)) { -msleep (PCIE_RESET_CONFIG_WAIT_MS ); -dev_dbg (dev,"Received Link up event. Starting enumeration!\n" ); -/* Rescan the bus to enumerate endpoint devices */ -pci_lock_rescan_remove (); -pci_rescan_bus (pp->bridge->bus); -pci_unlock_rescan_remove (); - } - } - -return IRQ_HANDLED ; -} -static irqreturn_t rockchip_pcie_ep_sys_irq_thread (int irq,void *arg) {struct rockchip_pcie *rockchip = arg;
@@ -568,29 +540,14 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)return IRQ_HANDLED ; } -static int rockchip_pcie_configure_rc (struct platform_device *pdev, -struct rockchip_pcie *rockchip) +static int rockchip_pcie_configure_rc (struct rockchip_pcie *rockchip) { -struct device *dev = &pdev->dev;struct dw_pcie_rp *pp; -int irq, ret;u32 val;if (!IS_ENABLED (CONFIG_PCIE_ROCKCHIP_DW_HOST ))return -ENODEV ; - irq =platform_get_irq_byname (pdev,"sys" ); -if (irq <0 ) -return irq; - - ret =devm_request_threaded_irq (dev, irq,NULL , - rockchip_pcie_rc_sys_irq_thread, -IRQF_ONESHOT ,"pcie-sys-rc" , rockchip); -if (ret) { -dev_err (dev,"failed to request PCIe sys IRQ\n" ); -return ret; - } -/* LTSSM enable control mode */ val =FIELD_PREP_WM16 (PCIE_LTSSM_ENABLE_ENHANCE ,1 );rockchip_pcie_writel_apb (rockchip, val,PCIE_CLIENT_HOT_RESET_CTRL );
@@ -602,17 +559,7 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev, pp = &rockchip->pci.pp; pp->ops = &rockchip_pcie_host_ops; - ret =dw_pcie_host_init (pp); -if (ret) { -dev_err (dev,"failed to initialize host\n" ); -return ret; - } - -/* unmask DLL up/down indicator */ - val =FIELD_PREP_WM16 (PCIE_RDLH_LINK_UP_CHGED ,0 ); -rockchip_pcie_writel_apb (rockchip, val,PCIE_CLIENT_INTR_MASK_MISC ); - -return ret; +return dw_pcie_host_init (pp); } static int rockchip_pcie_configure_ep(struct platform_device *pdev,
@@ -731,7 +678,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)switch (data->mode) {case DW_PCIE_RC_TYPE : - ret =rockchip_pcie_configure_rc (pdev, rockchip); + ret =rockchip_pcie_configure_rc (rockchip);if (ret) goto deinit_clk;break ;
Signature · 1 line
2.52.0
PATCH v2 3/6 Revert "PCI: qcom: Don't wait for link if we can detect Link Up"
In reply to Niklas CasselThis reverts commit 36971d6c5a9a134c15760ae9fd13c6d5f9a36abb.
While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
--- drivers/pci/controller/dwc/pcie-qcom.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/pci/controller/dwc/pcie-qcom.c +1 -4
@@ -1958,10 +1958,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)platform_set_drvdata (pdev, pcie); - irq =platform_get_irq_byname_optional (pdev,"global" ); -if (irq >0 ) - pp->use_linkup_irq = true; - ret =dw_pcie_host_init (pp);if (ret) {dev_err (dev,"cannot initialize host\n" );
@@ -1975,6 +1971,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; } + irq =platform_get_irq_byname_optional (pdev,"global" );if (irq >0 ) { ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, qcom_pcie_global_irq_thread,
Signature · 1 line
2.52.0
PATCH v2 4/6 Revert "PCI: qcom: Enable MSI interrupts together with Link up if 'Global IRQ' is supported"
In reply to Niklas CasselThis reverts commit ba4a2e2317b9faeca9193ed6d3193ddc3cf2aba3.
While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
--- drivers/pci/controller/dwc/pcie-qcom.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pci/controller/dwc/pcie-qcom.c +1 -3
@@ -136,7 +136,6 @@/* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */ #define PARF_INT_ALL_LINK_UP BIT(13) -#define PARF_INT_MSI_DEV_0_7 GENMASK(30, 23)/* PARF_NO_SNOOP_OVERRIDE register fields */ #define WR_NO_SNOOP_OVERRIDE_EN BIT(1)
@@ -1982,8 +1981,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; } -writel_relaxed (PARF_INT_ALL_LINK_UP |PARF_INT_MSI_DEV_0_7 , - pcie->parf +PARF_INT_ALL_MASK ); +writel_relaxed (PARF_INT_ALL_LINK_UP , pcie->parf +PARF_INT_ALL_MASK ); }qcom_pcie_icc_opp_update (pcie);
Signature · 1 line
2.52.0
Re: [PATCH v2 4/6] Revert "PCI: qcom: Enable MSI interrupts together with Link up if 'Global IRQ' is supported"
Quoted 62 lines · Niklas Cassel
This reverts commit
ba4a2e2317b9faeca9193ed6d3193ddc3cf2aba3.While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus0004:43:busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:43:busn_res: [bus 43-41] end is updated to 43pci_bus0004:43:busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for thempci_bus0004:44:busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:44:busn_res: [bus 44-41] end is updated to 44pci_bus0004:44:busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for thempci_bus0004:45:busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:45:busn_res: [bus 45-41] end is updated to 45pci_bus0004:45:busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for thempci_bus0004:46:busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:46:busn_res: [bus 46-41] end is updated to 46pci_bus0004:46:busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for thempci_bus0004:42:busn_res: [bus 42-41] end is updated to 46pci_bus0004:42:busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
Suggested-by Manivannan Sadhasivam <[email protected]>Acked-by Shawn Lin <[email protected]>Tested-by Shawn Lin <[email protected]>Signed-off-by Niklas Cassel <[email protected]>--- drivers/pci/controller/dwc/pcie-qcom.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)drivers/pci/controller/dwc/pcie-qcom.c +0
-1@@ -136,7 +136,6 @@/* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */ #define PARF_INT_ALL_LINK_UP BIT(13) -#define PARF_INT_MSI_DEV_0_7 GENMASK(30, 23)/* PARF_NO_SNOOP_OVERRIDE register fields */ #define WR_NO_SNOOP_OVERRIDE_EN BIT(1)
drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_probe(struct platform_device *pdev) · line 1986
Quoted 5 lines
drivers/pci/controller/dwc/pcie-qcom.c +0
-2@@ -1982,8 +1981,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; } -writel_relaxed (PARF_INT_ALL_LINK_UP |PARF_INT_MSI_DEV_0_7 , - pcie->parf +PARF_INT_ALL_MASK );
MSI's needs to be enabled irrespective of this series as part of global IRQ otherwise MSI's will not be triggered in few platforms.
Mani, exclude this patch while applying.
- Krishna Chaitanya
Quoted 4 lines
drivers/pci/controller/dwc/pcie-qcom.c +1
-0+writel_relaxed (PARF_INT_ALL_LINK_UP , pcie->parf +PARF_INT_ALL_MASK ); }qcom_pcie_icc_opp_update (pcie);
Re: [PATCH v2 4/6] Revert "PCI: qcom: Enable MSI interrupts together with Link up if 'Global IRQ' is supported"
Quoted 10 lines · Krishna Chaitanya Chundru
drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_probe(struct platform_device *pdev) · line 1985
Quoted 5 lines
drivers/pci/controller/dwc/pcie-qcom.c +0
-2@@ -1982,8 +1981,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; } -writel_relaxed (PARF_INT_ALL_LINK_UP |PARF_INT_MSI_DEV_0_7 , - pcie->parf +PARF_INT_ALL_MASK );MSI's needs to be enabled irrespective of this series as part of global IRQ otherwise
MSI's will not be triggered in few platforms.Mani, exclude this patch while applying.
From the commit message of the reverted patch:
The MSI bits in PARF_INT_ALL_MASK register are enabled
by default in the hardware, but commit 4581403f6792 ("PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt") disabled them and enabled only the Link up interrupt.
Thus, applying this patch should be safe, or at least bring back the original behavior as it was before the Link Up IRQ patches got applied.
But if you prefer to enable the MSI bits explicitly, in order to not rely on HW defaults, that should also be fine.
Kind regards,
Niklas
Manivannan Sadhasivam
maintainerRe: [PATCH v2 4/6] Revert "PCI: qcom: Enable MSI interrupts together with Link up if 'Global IRQ' is supported"
+ Qiang
Quoted 14 lines · Niklas Cassel
Quoted 10 lines · Krishna Chaitanya Chundru
drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_probe(struct platform_device *pdev) · line 1985
Quoted 5 lines
drivers/pci/controller/dwc/pcie-qcom.c +0
-2@@ -1982,8 +1981,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; } -writel_relaxed (PARF_INT_ALL_LINK_UP |PARF_INT_MSI_DEV_0_7 , - pcie->parf +PARF_INT_ALL_MASK );MSI's needs to be enabled irrespective of this series as part of global IRQ otherwise
MSI's will not be triggered in few platforms.Mani, exclude this patch while applying.
From the commit message of the reverted patch:
Quoted 9 lines
The MSI bits in
PARF_INT_ALL_MASKregister are enabled
by default in the hardware, but commit4581403f6792("PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt") disabled them and enabled only the Link up interrupt.Thus, applying this patch should be safe, or at least bring back the original behavior as it was before the Link Up IRQ patches got applied.
Yes!
Quoted 3 lines
But if you prefer to enable the MSI bits explicitly, in order to not rely on HW defaults, that should also be fine.
I believe Qiang reported internally that we need to explicitly enable PARF_INT_MSI_DEV_0_7 for all platforms as the hardware default is not always correct. So that should come as a separate patch.
- Mani
Signature · 1 line
மணிவண்ணன் சதாசிவம்
PATCH v2 5/6 Revert "PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt"
In reply to Niklas CasselThis reverts commit 4581403f67929d02c197cb187c4e1e811c9e762a.
While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
--- drivers/pci/controller/dwc/pcie-qcom.c | 58 +------------------------- 1 file changed, 1 insertion(+), 57 deletions(-)
drivers/pci/controller/dwc/pcie-qcom.c +1 -57
@@ -55,9 +55,6 @@#define PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1a8#define PARF_Q2A_FLUSH 0x1ac#define PARF_LTSSM 0x1b0 -#define PARF_INT_ALL_STATUS 0x224 -#define PARF_INT_ALL_CLEAR 0x228 -#define PARF_INT_ALL_MASK 0x22c#define PARF_SID_OFFSET 0x234#define PARF_BDF_TRANSLATE_CFG 0x24c#define PARF_DBI_BASE_ADDR_V2 0x350
@@ -134,9 +131,6 @@/* PARF_LTSSM register fields */ #define LTSSM_EN BIT(8) -/* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */ -#define PARF_INT_ALL_LINK_UP BIT(13) -/* PARF_NO_SNOOP_OVERRIDE register fields */ #define WR_NO_SNOOP_OVERRIDE_EN BIT(1)#define RD_NO_SNOOP_OVERRIDE_EN BIT(3)
@@ -1635,32 +1629,6 @@ static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie) qcom_pcie_link_transition_count); } -static irqreturn_t qcom_pcie_global_irq_thread (int irq,void *data) -{ -struct qcom_pcie *pcie = data; -struct dw_pcie_rp *pp = &pcie->pci->pp; -struct device *dev = pcie->pci->dev; -u32 status =readl_relaxed (pcie->parf +PARF_INT_ALL_STATUS ); - -writel_relaxed (status, pcie->parf +PARF_INT_ALL_CLEAR ); - -if (FIELD_GET (PARF_INT_ALL_LINK_UP , status)) { -msleep (PCIE_RESET_CONFIG_WAIT_MS ); -dev_dbg (dev,"Received Link up event. Starting enumeration!\n" ); -/* Rescan the bus to enumerate endpoint devices */ -pci_lock_rescan_remove (); -pci_rescan_bus (pp->bridge->bus); -pci_unlock_rescan_remove (); - -qcom_pcie_icc_opp_update (pcie); - }else { -dev_WARN_ONCE (dev,1 ,"Received unknown event. INT_STATUS: 0x%08x\n" , - status); - } - -return IRQ_HANDLED ; -} -static void qcom_pci_free_msi (void *ptr) {struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
@@ -1805,8 +1773,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)struct dw_pcie_rp *pp;struct resource *res;struct dw_pcie *pci; -int ret, irq; -char *name; +int ret; pcie_cfg =of_device_get_match_data (dev);if (!pcie_cfg) {
@@ -1963,27 +1930,6 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_phy_exit; } - name =devm_kasprintf (dev,GFP_KERNEL ,"qcom_pcie_global_irq%d" , -pci_domain_nr (pp->bridge->bus)); -if (!name) { - ret = -ENOMEM ; - goto err_host_deinit; - } - - irq =platform_get_irq_byname_optional (pdev,"global" ); -if (irq >0 ) { - ret =devm_request_threaded_irq (&pdev->dev, irq,NULL , - qcom_pcie_global_irq_thread, -IRQF_ONESHOT , name, pcie); -if (ret) { -dev_err_probe (&pdev->dev, ret, -"Failed to request Global IRQ\n" ); - goto err_host_deinit; - } - -writel_relaxed (PARF_INT_ALL_LINK_UP , pcie->parf +PARF_INT_ALL_MASK ); - } -qcom_pcie_icc_opp_update (pcie);if (pcie->mhi)
@@ -1991,8 +1937,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)return 0 ; -err_host_deinit: -dw_pcie_host_deinit (pp); err_phy_exit:list_for_each_entry_safe (port, tmp, &pcie->ports, list) {phy_exit (port->phy);
Signature · 1 line
2.52.0
Re: [PATCH v2 5/6] Revert "PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt"
Quoted 37 lines · Niklas Cassel
This reverts commit
4581403f67929d02c197cb187c4e1e811c9e762a.While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus0004:43:busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:43:busn_res: [bus 43-41] end is updated to 43pci_bus0004:43:busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for thempci_bus0004:44:busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:44:busn_res: [bus 44-41] end is updated to 44pci_bus0004:44:busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for thempci_bus0004:45:busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:45:busn_res: [bus 45-41] end is updated to 45pci_bus0004:45:busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for thempci_bus0004:46:busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:46:busn_res: [bus 46-41] end is updated to 46pci_bus0004:46:busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for thempci_bus0004:42:busn_res: [bus 42-41] end is updated to 46pci_bus0004:42:busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
Quoted 8 lines
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
Suggested-by Manivannan Sadhasivam <[email protected]>Acked-by Shawn Lin <[email protected]>Tested-by Shawn Lin <[email protected]>Signed-off-by Niklas Cassel <[email protected]>
Removing patch 3/6 should be sufficient, don't remove global IRQ patch, this will be helpful when endpoint is connected at later point of time.
- Krishna Chaitanya.
Quoted 108 lines
--- drivers/pci/controller/dwc/pcie-qcom.c | 58 +------------------------- 1 file changed, 1 insertion(+), 57 deletions(-)drivers/pci/controller/dwc/pcie-qcom.c +1
-57@@ -55,9 +55,6 @@#define PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1a8#define PARF_Q2A_FLUSH 0x1ac#define PARF_LTSSM 0x1b0 -#define PARF_INT_ALL_STATUS 0x224 -#define PARF_INT_ALL_CLEAR 0x228 -#define PARF_INT_ALL_MASK 0x22c#define PARF_SID_OFFSET 0x234#define PARF_BDF_TRANSLATE_CFG 0x24c#define PARF_DBI_BASE_ADDR_V2 0x350@@ -134,9 +131,6 @@/* PARF_LTSSM register fields */ #define LTSSM_EN BIT(8) -/* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */ -#define PARF_INT_ALL_LINK_UP BIT(13) -/* PARF_NO_SNOOP_OVERRIDE register fields */ #define WR_NO_SNOOP_OVERRIDE_EN BIT(1)#define RD_NO_SNOOP_OVERRIDE_EN BIT(3)@@ -1635,32 +1629,6 @@ static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie) qcom_pcie_link_transition_count); } -static irqreturn_t qcom_pcie_global_irq_thread (int irq,void *data) -{ -struct qcom_pcie *pcie = data; -struct dw_pcie_rp *pp = &pcie->pci->pp; -struct device *dev = pcie->pci->dev; -u32 status =readl_relaxed (pcie->parf +PARF_INT_ALL_STATUS ); - -writel_relaxed (status, pcie->parf +PARF_INT_ALL_CLEAR ); - -if (FIELD_GET (PARF_INT_ALL_LINK_UP , status)) { -msleep (PCIE_RESET_CONFIG_WAIT_MS ); -dev_dbg (dev,"Received Link up event. Starting enumeration!\n" ); -/* Rescan the bus to enumerate endpoint devices */ -pci_lock_rescan_remove (); -pci_rescan_bus (pp->bridge->bus); -pci_unlock_rescan_remove (); - -qcom_pcie_icc_opp_update (pcie); - }else { -dev_WARN_ONCE (dev,1 ,"Received unknown event. INT_STATUS: 0x%08x\n" , - status); - } - -return IRQ_HANDLED ; -} -static void qcom_pci_free_msi (void *ptr) {struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;@@ -1805,8 +1773,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)struct dw_pcie_rp *pp;struct resource *res;struct dw_pcie *pci; -int ret, irq; -char *name; +int ret; pcie_cfg =of_device_get_match_data (dev);if (!pcie_cfg) {@@ -1963,27 +1930,6 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_phy_exit; } - name =devm_kasprintf (dev,GFP_KERNEL ,"qcom_pcie_global_irq%d" , -pci_domain_nr (pp->bridge->bus)); -if (!name) { - ret = -ENOMEM ; - goto err_host_deinit; - } - - irq =platform_get_irq_byname_optional (pdev,"global" ); -if (irq >0 ) { - ret =devm_request_threaded_irq (&pdev->dev, irq,NULL , - qcom_pcie_global_irq_thread, -IRQF_ONESHOT , name, pcie); -if (ret) { -dev_err_probe (&pdev->dev, ret, -"Failed to request Global IRQ\n" ); - goto err_host_deinit; - } - -writel_relaxed (PARF_INT_ALL_LINK_UP , pcie->parf +PARF_INT_ALL_MASK ); - } -qcom_pcie_icc_opp_update (pcie);if (pcie->mhi)@@ -1991,8 +1937,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)return 0 ; -err_host_deinit: -dw_pcie_host_deinit (pp); err_phy_exit:list_for_each_entry_safe (port, tmp, &pcie->ports, list) {phy_exit (port->phy);
Re: [PATCH v2 5/6] Revert "PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt"
Hello Krishna,
Quoted 3 lines · Krishna Chaitanya Chundru
Removing patch 3/6 should be sufficient, don't remove global IRQ patch, this will be helpful
when endpoint is connected at later point of time.
Please see Mani's reply here:
https://lore.kernel.org/linux-pci/fle74skju2rorxmfdvosmeyrx3g75rysuszov5ofvde2exj4ir@3kfjyfyhczmn/
"And neither the controller driver."
Sounds to me like he still wants this patch (which removes the support from the controller driver).
Kind regards,
Niklas
Manivannan Sadhasivam
maintainerRe: [PATCH v2 5/6] Revert "PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt"
Quoted 7 lines · Niklas Cassel
Hello Krishna,
Quoted 3 lines · Krishna Chaitanya Chundru
Removing patch 3/6 should be sufficient, don't remove global IRQ patch, this will be helpful
when endpoint is connected at later point of time.
Quoted 8 lines
Please see Mani's reply here:
https://lore.kernel.org/linux-pci/fle74skju2rorxmfdvosmeyrx3g75rysuszov5ofvde2exj4ir@3kfjyfyhczmn/"And neither the controller driver."
Sounds to me like he still wants this patch (which removes the support from the controller driver).
Yes, allowing hotplug for a non-hotplug Root Port is a bad idea. Too bad that I proposed it in the first place... Let's revert for all platforms to avoid setting bad precedence.
- Mani
Signature · 1 line
மணிவண்ணன் சதாசிவம்
PATCH v2 6/6 Revert "PCI: dwc: Don't wait for link up if driver can detect Link Up event"
In reply to Niklas CasselThis reverts commit 8d3bf19f1b585a3cc0027f508b64c33484db8d0d.
While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
--- drivers/pci/controller/dwc/pcie-designware-host.c | 10 ++-------- drivers/pci/controller/dwc/pcie-designware.h | 1 - 2 files changed, 2 insertions(+), 9 deletions(-)
drivers/pci/controller/dwc/pcie-designware-host.c +2 -8
@@ -665,14 +665,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) goto err_remove_edma; } -/* -* Note: Skip the link up delay only when a Link Up IRQ is present. -* If there is no Link Up IRQ, we should not bypass the delay -* because that would require users to manually rescan for devices. -*/ -if (!pp->use_linkup_irq) -/* Ignore errors, the link may come up later */ -dw_pcie_wait_for_link (pci); +/* Ignore errors, the link may come up later */ +dw_pcie_wait_for_link (pci); ret =pci_host_probe (bridge);if (ret)
drivers/pci/controller/dwc/pcie-designware.h +0 -1
@@ -438,7 +438,6 @@ struct dw_pcie_rp {bool use_atu_msg;int msg_atu_index;struct resource *msg_res; -bool use_linkup_irq;struct pci_eq_presets presets;struct pci_config_window *cfg;bool ecam_enabled;
Signature · 1 line
2.52.0
Manivannan Sadhasivam
AppliedQuoted 26 lines · Niklas Cassel
Revert all patches related to pcie-designware Root Complex Link Up IRQ support.
While this fake hotplugging was a nice idea, it has shown that this feature
does not handle PCIe switches correctly:pci_bus0004:43:busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:43:busn_res: [bus 43-41] end is updated to 43pci_bus0004:43:busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for thempci_bus0004:44:busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:44:busn_res: [bus 44-41] end is updated to 44pci_bus0004:44:busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for thempci_bus0004:45:busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:45:busn_res: [bus 45-41] end is updated to 45pci_bus0004:45:busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for thempci_bus0004:46:busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41])pci_bus0004:46:busn_res: [bus 46-41] end is updated to 46pci_bus0004:46:busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for thempci_bus0004:42:busn_res: [bus 42-41] end is updated to 46pci_bus0004:42:busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
Quoted 1 line
[...]
Applied, thanks!
[1/6] Revert "PCI: dw-rockchip: Don't wait for link since we can detect Link Up"
commit: fc6298086bfacaa7003b0bd1da4e4f42b29f7d77
[2/6] Revert "PCI: dw-rockchip: Enumerate endpoints based on dll_link_up IRQ"
commit: 180c3cfe36786d261a55da52a161f9e279b19a6f
[3/6] Revert "PCI: qcom: Don't wait for link if we can detect Link Up"
commit: e9ce5b3804436301ab343bc14203a4c14b336d1b
[4/6] Revert "PCI: qcom: Enable MSI interrupts together with Link up if 'Global IRQ' is supported"
commit: 7ebdefb87942073679e56cfbc5a72e8fc5441bfc
[5/6] Revert "PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt"
commit: 9a9793b55854422652ea92625e48277c4651c0fd
[6/6] Revert "PCI: dwc: Don't wait for link up if driver can detect Link Up event"
commit: 142d5869f6eec3110adda0ad2d931f5b3c22371dBest regards,
Signature · 1 line
Manivannan Sadhasivam <[email protected]>