PCI: rcar-gen4: Recover from link down and route Root Port interrupts
Subsystems (6)
v5 cover PCI: rcar-gen4: Recover from link down and route Root Port interrupts
Hi,
This series improves error handling in the pcie-rcar-gen4 driver, which supports PCIe controllers in R-Car Gen4 SoCs and the PCIe4 controller in R-Car Gen5 SoCs. It fixes unexpected link-down handling so the host doesn't hang, and wires up missing Root Port interrupts (AER, PME, bandwidth notifications) so port services actually work. I tested on an R-Car S4 Spider (r8a779f0); Marek tested the link state fix and the Root Port AER and bandwidth paths on an R-Car V4H Sparrow Hawk.
A few hardware quirks made this tricky:
1. PCIEINTSTS0 link-up bits don't track the actual link state. The driver never noticed when the link dropped and kept trying config accesses on the dead link. Patch 3 combines the APP link-up event check from Figure 104.5 with the PORT_LINK_DEBUG1 live link check, which patch 2 factors out of the DWC core. Startup clears the APP latches before enabling LTSSM, and the DWC core polls the combined condition on the RC side.
2. On S4, a DBI access immediately after an unexpected link down can hang the host. Commit 0056d29f8c1b ("PCI: rcar-gen4: Assure reset occurs before DBI access") describes an SError on V4H after reset deassertion, but whether the S4 hang shares the same underlying cause has not been verified.
Adding a delay before the DBI access avoided the hang in my tests, but that alone would not provide link-down recovery. Also, the reset request shares intreq_pcim_sub with iMSI-RX, while AER arrives on another IRQ. Delaying AER dispatch alone would therefore leave the MSI handler exposed, and both paths would still need coordination with the controller reset.
The driver handles the reset request itself and schedules pci_host_handle_link_down(), as the rockchip and qcom drivers do. This also provides recovery with older DTs that have no "aer" interrupt, without relying on AER to initiate it. The reset callback shares the reset sequence used at probe, including the reset-status readback and delay added by 0056d29f8c1b.
This covers the interrupt paths that fire on link down. Port service threads or work already queued or running at that point are not stopped.
3. Root Port interrupts only trigger APP status bits on platform IRQs. The controller appears to lack SII2MSI, so Root Port MSIs never reach the GIC ITS. The Root Port's INTx is routed to intreq_pcim_sub as well, which the driver holds, so port services can't request it either. This series works around it by hiding Root Port MSI caps across the board and emulating INTx using a virtual IRQ domain, fed by the "aer" IRQ and intreq_pcim_sub.
Patch 1 is only loosely related: it adds Renesas to the RAS DES VSEC list so the DWC debugfs error injection works on R-Car. I used it to test the Root Port AER path (see below) and included it here for that reason. Happy to send it separately if preferred.
Note: backward compatibility with older DTs is kept. Without the "aer"
interrupt, only Root Port AER remains unavailable. See the testing section below.
Dependencies
------------
- [PATCH 0/2] PCI: dwc: Clean up DWC PCIe register macros
https://lore.kernel.org/r/[email protected]/
(*already landed in controller/dwc and pci/next)
- [PATCH v5] PCI: rcar-gen4: Add missing PM ops
https://lore.kernel.org/r/[email protected]/
(*already landed in controller/dwc-rcar-gen4 and pci/next)
- [PATCH] dt-bindings: PCI: rcar-gen4-pci-host: Replace sft_ce IRQ with AER IRQ on R-Car Gen5
https://lore.kernel.org/r/[email protected]/
(*under review)
Applies cleanly to -next (e.g. next-20261014) with the latter two prerequisite patches applied first.
Proposed merge plan
-------------------
With v5, this series seems to have almost converged, so I'd like to ask how maintainers would prefer to merge it. Any advice would be greatly appreciated. Here's what I have in mind:
To Mani:
I'd expect patches 1-14 to go through the PCI tree. One option would be a two-stage approach like:
1. Take patches 1/2/5/10 through controller/dwc ("PCI: dwc:" patches).
2. Once those patches are available in controller/dwc-rcar-gen4, take the remaining patches 3/4/6-9/11-14 there
(the R-Car driver and binding patches).
To Geert:
Once patch 4 is available in renesas-devel, I'd like to ask you to take patches 15-17 together.
Re-tested with v5
-----------------
Setup: R-Car S4 Spider (RC) linked to another S4 Spider running the
pci-epf-test endpoint. pci_endpoint_test is bound on the RC side.
1. Link down / recovery. On the EP side, toggle the endpoint controller off and on. The short pause keeps the endpoint away long enough for the RC to notice, but brings it back within the reset window so recovery can succeed. Adopted the test approach from [1]. Make sure that the unused function 0000:01:00.1 was removed on the RC before testing.
# cd /sys/kernel/config/pci_ep
# echo 0 > controllers/e65d0000.pcie-ep/start # sleep 0.1
# echo 1 > controllers/e65d0000.pcie-ep/start
Expected on the RC dmesg:
pcieport 0000:00:00.0: Recovering Root Port due to Link Down pcieport 0000:00:00.0: Root Port has been reset pcieport 0000:00:00.0: AER: device recovery successful
and the "msi" (intreq_pcim_sub) interrupt count going up in /proc/interrupts. Without this series nothing shows up here: the link comes back on its own once the endpoint returns, but the RC never notices the outage and the endpoint is left unconfigured (see 4). Config accesses issued while the link is down hang the host.
[1] https://lore.kernel.org/r/abFMa6DCGGLUHddA@fedora/
2. Bandwidth notification. On the RC, retrain the link:
# setpci -s 00:00.0 CAP_EXP+0x10.w=0x0c23
Expected:
- the virtual Root Port IRQ (rcar-gen4-rp in /proc/interrupts, shared by PCIe PME, aerdrv and PCIe bwctrl) fires once
- bwctrl clears LnkSta.LBMS
(setpci -s 00:00.0 CAP_EXP+0x12.w reads 0x2024 again).
Before the series LnkSta read 0xe024 afterwards, LBMS and LABS stuck.
This test does not generate a PME. The PME service shares the virtual IRQ, but its path was not exercised separately.
3. Root Port AER. On the RC, inject an LCRC error with the DWC debugfs (patch 1) and issue one config read so a TLP actually goes out:
# cd /sys/kernel/debug/dwc_pcie_e65d0000.pcie/rasdes_err_inj
# echo 1 > rx_lcrc # error detected by the Root Port
# setpci -s 01:00.0 VENDOR_ID.w
# echo 1 > tx_lcrc # error detected by the endpoint,
# setpci -s 01:00.0 VENDOR_ID.w # reported back with ERR_COR
Expected on the RC dmesg, respectively:
pcieport 0000:00:00.0: PCIe Bus Error: severity=Correctable
pcieport 0000:00:00.0: [ 6] BadTLP | Receiver | Data Link Layer
pcieport 0000:00:00.0: AER: Correctable Error message received from 0000:01:00.0 pci-endpoint-test 0000:01:00.0: PCIe Bus Error: severity=Correctable
pci-endpoint-test 0000:01:00.0: [ 6] BadTLP | Receiver | Data Link Layer
plus the virtual Root Port IRQ count and aer_rootport_total_err_cor going up by one each time. The link stays up throughout, the DLL retry recovers the TLP. Before the series nothing is reported.
4. Regression check. Run pci_endpoint_test after step 1. PASS/FAIL/SKIP counts match a run without step 1.
Configurations:
a. Without this series**
b. GIC ITS, DT with the new "aer" interrupt (this series) c. GIC ITS, DT without "aer" (b43aa6a6ebe8 ("arm64: dts: renesas:
r8a779f0: Add GICv3 ITS and update PCIe nodes") or later)
d. iMSI-RX, DT without msi-parent, msi-map or "aer"
Result:
recovery bwctrl RP AER pcitest
---------------------------------------------------------
a. none no no all FAIL b. ok ok ok no change c. ok ok n/a* no change d. ok ok n/a* no change
* Root Port AER needs the "aer" interrupt; without it the behaviour is unchanged from before the series.
** Only patch 1 applied on top of the base, so the same debugfs error
injection could be used for the comparison. pci_endpoint_test fails across the board there because nothing restores the endpoint after the toggle.
Best regards,
Koichiro
---
Changes in v5:
- Drop no_rp_msi patch; handle it in R-Car host setup instead. (Shawn)
- Use scoped_guard() in the MSI setup error path. (Marek)
- Collect R-b tags from Marek, Shawn and Rob. Thanks for reviewing!
Changes in v4:
- Rebase onto next-20261014 with Marek's Gen5 AER binding fix. Only patch 4 needed changes.
Changes in v3:
- Rebase onto next-20261002 with Marek's PM ops v5 applied first.
- Keep parent IRQs registered across suspend/resume to fix the unbalanced enable Geert reported. Request them in probe with IRQF_NO_AUTOEN and enable/disable them in .init()/.deinit().
- Use a freezable workqueue for recovery and reject suspend while reinitialization is pending. Re-arm Root Port events only after resume has set up the link.
- Factor out the PORT_LINK_DEBUG1 check (patch 2) and reuse it in patch 3. (Marek)
- Replace .reinit() with .configure, folding v2 patch 8 into patch 9. Drop the reset mutex and rely on PCI core serialization. (Marek)
- Free the MSI domain after host .deinit(), so the driver can stop its parent IRQ first (patch 10). This closes the known gap in v2.
- Reset the controller if recovery finds no Root Port, so a successful reset can restore interrupt delivery.
- Drop the raw lock around virtual IRQ dispatch. Preserve MSI-form latches in irq_ack() while the IRQ is in progress or disabled, avoiding lost notifications without nesting port service locks.
- Add patch 7's Fixes tag, clarify SoC generations, separate guard() from goto-based cleanup, and simplify IRQ return statements. (Marek, Sashiko)
- Add the V4H and V4M DTS patches 17 and 18. (Marek)
- Collect Reviewed-by (Marek, Krzysztof) and Tested-by (Marek) tags. Drop Marek's Reviewed-by from the reworked patches 12 and 14.
Changes in v2:
- Rebased onto next-20260925, including Marek's R-Car X5H support.
- Keep .link_up() and require both the APP link-up events and the PORT_DEBUG1 live link check. Clear the APP latches before enabling LTSSM and reuse the DWC core's polling on the RC side. (Marek)
- Let the R-Car driver own intreq_pcim_sub in all configurations (new patch 10) and check the reset request from its own handler instead of hooking into the DWC chained handler. Replace the pre_msi_irq host op from v1 with an export of dw_handle_msi_irq() (patch 4). (Marek)
- Keep the reset_control_status() check before asserting the power reset. (Marek)
- Separate controller reinitialization from .init()/.deinit() and split out preparatory changes.
- Use bool flags instead of a state bitmask. (Marek)
- Keep Root Port AER notifications masked until .post_init, after enumeration. In v1, they could be enabled during port-service probing.
- Keep APP interrupt sources masked if the Root Port reset callback fails, regardless of the reset trigger.
- Hold a reference on the Root Port in the link-down recovery work.
- Clear only the MSI-form Root Port latches through PCIEINTSTS0CLR; the INTx bits are reserved there.
- Serialize dispatches to the virtual Root Port IRQ from its two parent interrupts.
- Collected Marek's Reviewed-by on patches 1, 3 and 15.
v4: https://lore.kernel.org/r/[email protected]/
v3: https://lore.kernel.org/r/[email protected]/
v2: https://lore.kernel.org/r/[email protected]/
v1: https://lore.kernel.org/r/[email protected]/
Koichiro Den (17):
PCI: dwc: Add Renesas to the RAS DES VSEC list
PCI: dwc: Factor out the PORT_LINK_DEBUG1 link-up check
PCI: rcar-gen4: Check live link status in link_up()
dt-bindings: PCI: rcar-gen4: Add optional "aer" interrupt for R-Car
Gen4 SoCs
PCI: dwc: Export dw_handle_msi_irq()
PCI: rcar-gen4: Move deinitialization helpers before SoC
initialization
PCI: rcar-gen4: Assert resets when Gen5 SoC PHY initialization fails
PCI: rcar-gen4: Separate hardware setup from resource acquisition
PCI: rcar-gen4: Add Root Port reset support
PCI: dwc: Free the MSI domain after the host .deinit() callback
PCI: rcar-gen4: Take over the iMSI-RX interrupt
PCI: rcar-gen4: Recover the Root Port on link down
PCI: rcar-gen4: Route Root Port AER to a virtual Root Port IRQ
PCI: rcar-gen4: Route Root Port PME and bandwidth notifications
arm64: dts: renesas: r8a779f0: Describe the PCIe AER interrupts
arm64: dts: renesas: r8a779g0: Describe the PCIe AER interrupts
arm64: dts: renesas: r8a779h0: Describe the PCIe AER interrupt
.../bindings/pci/rcar-gen4-pci-host.yaml | 13 +- arch/arm64/boot/dts/renesas/r8a779f0.dtsi | 10 +- arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 10 +- arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 5 +- .../pci/controller/dwc/pcie-designware-host.c | 32 +- drivers/pci/controller/dwc/pcie-designware.c | 15 +- drivers/pci/controller/dwc/pcie-designware.h | 1 + drivers/pci/controller/dwc/pcie-rcar-gen4.c | 725 ++++++++++++++++-- include/linux/pcie-dwc.h | 2 + 9 files changed, 721 insertions(+), 92 deletions(-)
base-commit: d419d682257d813f6cb7a28e42b35d0edf4ad038
prerequisite-patch-id: 5b4c9da1333342baa90cf6a86dc879e8382d3f65
prerequisite-patch-id: 258bbb5c2aaf676b727308d4e97d52f9c313eec0
Signature · 1 line
2.54.0
PATCH v5 1/17 PCI: dwc: Add Renesas to the RAS DES VSEC list
The Root Port on R-Car Gen4 SoCs (DWC 5.20a) carries the RAS DES capability with the stock DesignWare VSEC ID (0x02, rev 4), but vendor ID 0x1912 is not in dwc_pcie_rasdes_vsec_ids[]. Add Renesas to the list.
---
Changes since v3:
- No changes.
include/linux/pcie-dwc.h | 2 ++ 1 file changed, 2 insertions(+)
include/linux/pcie-dwc.h +2 -0
@@ -38,6 +38,8 @@ static const struct dwc_pcie_vsec_id dwc_pcie_rasdes_vsec_ids[] = { .vsec_id = 0x02, .vsec_rev = 0x4 }, { .vendor_id = PCI_VENDOR_ID_LECARC, .vsec_id = 0x02, .vsec_rev = 0x4 }, + { .vendor_id = PCI_VENDOR_ID_RENESAS, + .vsec_id = 0x02, .vsec_rev = 0x4 }, {} };
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 01/17] PCI: dwc: Add Renesas to the RAS DES VSEC list
Sashiko has reviewed this patch and found no issues.
Quoted 6 lines
The Root Port on R-Car Gen4 SoCs (DWC 5.20a) carries the RAS DES capability with the stock DesignWare VSEC ID (
0x02, rev 4), but vendor ID0x1912is not in dwc_pcie_rasdes_vsec_ids[]. Add Renesas to the list.Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH v5 2/17 PCI: dwc: Factor out the PORT_LINK_DEBUG1 link-up check
In reply to Koichiro Dendw_pcie_link_up() reads PORT_LINK_DEBUG1 when the glue driver provides no .link_up() callback. A callback that needs a platform-specific check on top of that register would have to duplicate the register read.
Factor the check out into dw_pcie_link_up_debug_check() and export it for modular glue drivers. pcie-rcar-gen4 will be the first user: its APP link-up events remain latched after the link goes down, so it has to combine them with the live PORT_LINK_DEBUG1 state.
No functional change.
---
Changes since v4:
- Add Reviewed-by tags from Shawn and Marek. No code changes.
drivers/pci/controller/dwc/pcie-designware.c | 15 +++++++++++---- drivers/pci/controller/dwc/pcie-designware.h | 1 + 2 files changed, 12 insertions(+), 4 deletions(-)
drivers/pci/controller/dwc/pcie-designware.c +11 -4
@@ -817,17 +817,24 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci) }EXPORT_SYMBOL_GPL (dw_pcie_wait_for_link); -bool dw_pcie_link_up (struct dw_pcie *pci) +/* Link state from PORT_LINK_DEBUG1: up and not in training */ +bool dw_pcie_link_up_debug_check (struct dw_pcie *pci) {u32 val; -if (pci->ops && pci->ops->link_up) -return pci->ops->link_up (pci); - val =dw_pcie_readl_dbi (pci,PORT_LINK_DEBUG1 );return ((val &PORT_LINK_DEBUG1_LINK_UP ) && (!(val &PORT_LINK_DEBUG1_LINK_IN_TRAINING ))); } +EXPORT_SYMBOL_GPL (dw_pcie_link_up_debug_check); + +bool dw_pcie_link_up (struct dw_pcie *pci) +{ +if (pci->ops && pci->ops->link_up) +return pci->ops->link_up (pci); + +return dw_pcie_link_up_debug_check (pci); +}EXPORT_SYMBOL_GPL (dw_pcie_link_up);void dw_pcie_upconfig_setup (struct dw_pcie *pci)
drivers/pci/controller/dwc/pcie-designware.h +1 -0
@@ -618,6 +618,7 @@ int dw_pcie_write(void __iomem *addr, int size, u32 val);u32 dw_pcie_read_dbi (struct dw_pcie *pci,u32 reg,size_t size);void dw_pcie_write_dbi (struct dw_pcie *pci,u32 reg,size_t size,u32 val);void dw_pcie_write_dbi2 (struct dw_pcie *pci,u32 reg,size_t size,u32 val); +bool dw_pcie_link_up_debug_check (struct dw_pcie *pci);bool dw_pcie_link_up (struct dw_pcie *pci);void dw_pcie_upconfig_setup (struct dw_pcie *pci);int dw_pcie_wait_for_link (struct dw_pcie *pci);
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 02/17] PCI: dwc: Factor out the PORT_LINK_DEBUG1 link-up check
Sashiko has reviewed this patch and found no issues.
Quoted 9 lines
dw_pcie_link_up()readsPORT_LINK_DEBUG1when the glue driver provides no .link_up() callback. A callback that needs a platform-specific check on top of that register would have to duplicate the register read.Factor the check out into
dw_pcie_link_up_debug_check()and export it for modular glue drivers. pcie-rcar-gen4 will be the first user: its APP link-up events remain latched after the link goes down, so it has to combine them with the livePORT_LINK_DEBUG1state.
Quoted 6 lines
No functional change.
Suggested-by Marek Vasut <[email protected]>Reviewed-by Shawn Lin <[email protected]>Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH v5 3/17 PCI: rcar-gen4: Check live link status in link_up()
In reply to Koichiro Denrcar_gen4_pcie_link_up() checks link state using SMLH_LINK_UP and RDLH_LINK_UP in PCIEINTSTS0. However, these bits do not reflect the live link state. On an R-Car S4, after taking down the endpoint, a link-down interrupt saw PCIEINTSTS0 = 0x20a000c5 with both bits still set. Even after resetting the controller with the LTSSM back in Polling, they read 0xa000c5, still set.
As a result, dw_pcie_link_up() keeps reporting the link as up after it has gone down. That defeats the check in dw_pcie_other_conf_map_bus(), which is supposed to stop config accesses to downstream devices while the link is down, so such accesses go out on the dead link and stall the host. It also makes the callback useless for the link-down recovery added later, which has to wait for the link to actually come back after resetting the controller.
Keep the APP link-up event check from Figure 104.5 of the R-Car S4 reference manual, but also require PORT_LINK_DEBUG1 to report the link up and not in training, through dw_pcie_link_up_debug_check(). The callback then rejects a downed link even if the APP link-up events remain latched.
Clear the APP latches before enabling LTSSM to discard events from a previous start, and only read them in .link_up(). RC startup uses dw_pcie_wait_for_link() to poll the combined condition.
---
Changes since v3:
- No changes.
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/pci/controller/dwc/pcie-rcar-gen4.c +10 -1
@@ -53,6 +53,9 @@#define PCIEDMAINTSTSEN 0x0314#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0) +/* PCIe Interrupt Status 0 Clear */ +#define PCIEINTSTS0CLR 0x0340 +/* Port Logic Registers 89 */ #define PRTLGC89 0x0b70
@@ -120,8 +123,11 @@ static bool rcar_gen4_pcie_link_up(struct dw_pcie *dw) val =readl (rcar->base +PCIEINTSTS0 ); mask =RDLH_LINK_UP |SMLH_LINK_UP ; +if ((val & mask) != mask) +return false; -return (val & mask) == mask; +/* The APP link-up events remain latched after the link goes down. */ +return dw_pcie_link_up_debug_check (dw); }/*
@@ -208,6 +214,9 @@ static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)struct rcar_gen4_pcie *rcar =to_rcar_gen4_pcie (dw);int ret; +/* Clear link-up events from the previous start. */ +writel (RDLH_LINK_UP |SMLH_LINK_UP , rcar->base +PCIEINTSTS0CLR ); +if (rcar->drvdata->ltssm_control) { ret = rcar->drvdata->ltssm_control (rcar, true);if (ret)
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 03/17] PCI: rcar-gen4: Check live link status in link_up()
Sashiko has reviewed this patch and found no issues.
Quoted 25 lines
rcar_gen4_pcie_link_up()checks link state usingSMLH_LINK_UPandRDLH_LINK_UPin PCIEINTSTS0. However, these bits do not reflect the live link state. On an R-Car S4, after taking down the endpoint, a link-down interrupt saw PCIEINTSTS0 =0x20a000c5with both bits still set. Even after resetting the controller with the LTSSM back in Polling, they read0xa000c5, still set.As a result,
dw_pcie_link_up()keeps reporting the link as up after it has gone down. That defeats the check indw_pcie_other_conf_map_bus(), which is supposed to stop config accesses to downstream devices while the link is down, so such accesses go out on the dead link and stall the host. It also makes the callback useless for the link-down recovery added later, which has to wait for the link to actually come back after resetting the controller.Keep the APP link-up event check from Figure 104.5 of the R-Car S4 reference manual, but also require
PORT_LINK_DEBUG1to report the link up and not in training, throughdw_pcie_link_up_debug_check(). The callback then rejects a downed link even if the APP link-up events remain latched.Clear the APP latches before enabling LTSSM to discard events from a previous start, and only read them in .link_up(). RC startup uses
dw_pcie_wait_for_link()to poll the combined condition.
Quoted 5 lines
Fixes 0d0c551011df ("PCI: rcar-gen4: Add R-Car Gen4 PCIe controller support for host mode")Suggested-by Marek Vasut <[email protected]>Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
PATCH v5 4/17 dt-bindings: PCI: rcar-gen4: Add optional "aer" interrupt for R-Car Gen4 SoCs
In reply to Koichiro DenThe intreq_pcim_lperr interrupt carries Root Port AER notifications, separately from the integrated MSI receiver interrupt. Describe it with the "aer" name so the controller driver can use it for Root Port error reporting.
Keep the fifth interrupt optional on R-Car Gen4 SoCs so existing device trees remain valid. Extend the example accordingly.
---
Changes since v4:
- Add Rob's Reviewed-by tag. No code changes.
v4: https://lore.kernel.org/r/[email protected]/
.../devicetree/bindings/pci/rcar-gen4-pci-host.yaml | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml +10 -3
@@ -26,7 +26,8 @@ properties: - const: renesas,rcar-gen5-pcie4 # R-Car Gen5 PCIe4 interrupts: - maxItems: 4 + minItems: 4 + maxItems: 5 clocks: maxItems: 2
@@ -93,11 +94,13 @@ allOf: - const: config interrupt-names: + minItems: 4 items: - const: msi - const: dma - const: sft_ce - const: app + - const: aer - if: properties: compatible:
@@ -117,6 +120,9 @@ allOf: - const: app - const: config + interrupts: + maxItems: 4 + interrupt-names: items: - const: msi
@@ -146,8 +152,9 @@ examples: interrupts = <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>, - <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "msi", "dma", "sft_ce", "app"; + <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi", "dma", "sft_ce", "app", "aer"; clocks = <&cpg CPG_MOD 624>, <&pcie0_clkref>; clock-names = "core", "ref"; power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 04/17] dt-bindings: PCI: rcar-gen4: Add optional "aer" interrupt for R-Car Gen4 SoCs
Sashiko has reviewed this patch and found no issues.
Quoted 10 lines
The
intreq_pcim_lperrinterrupt carries Root Port AER notifications, separately from the integrated MSI receiver interrupt. Describe it with the "aer" name so the controller driver can use it for Root Port error reporting.Keep the fifth interrupt optional on R-Car Gen4 SoCs so existing device trees remain valid. Extend the example accordingly.
Reviewed-by Rob Herring (Arm) <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Re: [PATCH v5 04/17] dt-bindings: PCI: rcar-gen4: Add optional "aer" interrupt for R-Car Gen4 SoCs
In reply to Koichiro DenQuoted 10 lines · Koichiro Den
The
intreq_pcim_lperrinterrupt carries Root Port AER notifications, separately from the integrated MSI receiver interrupt. Describe it with the "aer" name so the controller driver can use it for Root Port error reporting.Keep the fifth interrupt optional on R-Car Gen4 SoCs so existing device trees remain valid. Extend the example accordingly.
Reviewed-by Rob Herring (Arm) <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Thank you !
PATCH v5 5/17 PCI: dwc: Export dw_handle_msi_irq()
In reply to Koichiro DenNot every platform brings the iMSI-RX interrupt out as a dedicated line. On spear13xx and keembay it is a status bit in the SoC wrapper's interrupt register that the glue driver has to check and clear, and on dra7xx it shares the line with other sources the glue driver has to demultiplex. These drivers set msi_irq[0] to -ENODEV so the core does not install its chained handler, and service the MSIs from a handler of their own. spear13xx and keembay call dw_handle_msi_irq() for that. Both are built-in, so the function was never exported. dra7xx can be a module and has an MSI loop of its own.
Export the function so that modular glue drivers can use it as well. The pcie-rcar-gen4 driver, where both situations apply, is the first user.
---
Changes since v4:
- Add Shawn's Reviewed-by tag. No code changes.
v4: https://lore.kernel.org/r/[email protected]/
drivers/pci/controller/dwc/pcie-designware-host.c | 1 + 1 file changed, 1 insertion(+)
drivers/pci/controller/dwc/pcie-designware-host.c +1 -0
@@ -85,6 +85,7 @@ void dw_handle_msi_irq(struct dw_pcie_rp *pp)generic_handle_demux_domain_irq (pp->irq_domain, irq_off + pos); } } +EXPORT_SYMBOL_GPL (dw_handle_msi_irq);/* Chained MSI interrupt service routine */ static void dw_chained_msi_isr (struct irq_desc *desc)
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 05/17] PCI: dwc: Export dw_handle_msi_irq()
Sashiko has reviewed this patch and found no issues.
Quoted 10 lines
Not every platform brings the iMSI-RX interrupt out as a dedicated line. On spear13xx and keembay it is a status bit in the SoC wrapper's interrupt register that the glue driver has to check and clear, and on dra7xx it shares the line with other sources the glue driver has to demultiplex. These drivers set msi_irq[0] to
-ENODEVso the core does not install its chained handler, and service the MSIs from a handler of their own. spear13xx and keembay calldw_handle_msi_irq()for that. Both are built-in, so the function was never exported. dra7xx can be a module and has an MSI loop of its own.
Quoted 6 lines
Export the function so that modular glue drivers can use it as well. The pcie-rcar-gen4 driver, where both situations apply, is the first user.
Reviewed-by Marek Vasut <[email protected]>Reviewed-by Shawn Lin <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
PATCH v5 6/17 PCI: rcar-gen4: Move deinitialization helpers before SoC initialization
In reply to Koichiro DenMove common_deinit() next to common_init(), ahead of rcar_gen5_pcie_init(), which will call it when PHY initialization fails. Place rcar_gen5_pcie_deinit() before rcar_gen5_pcie_init() as well.
Keep these moves separate from the following initialization refactoring to make that patch easier to review. The function bodies are unchanged.
No functional changes.
---
Changes since v3:
- No changes.
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 30 ++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-)
drivers/pci/controller/dwc/pcie-rcar-gen4.c +15 -15
@@ -301,6 +301,15 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar)return ret; } +static void rcar_gen4_pcie_common_deinit (struct rcar_gen4_pcie *rcar) +{ +struct dw_pcie *dw = &rcar->dw; + +reset_control_assert (dw->core_rsts[DW_PCIE_PWR_RST ].rstc); +reset_control_assert (dw->core_rsts[DW_PCIE_CORE_RST ].rstc); +clk_bulk_disable_unprepare (DW_PCIE_NUM_CORE_CLKS , dw->core_clks); +} +static int rcar_gen4_v4h_v4m_pcie_init (struct rcar_gen4_pcie *rcar) {struct dw_pcie *dw = &rcar->dw;
@@ -326,6 +335,12 @@ static int rcar_gen4_v4h_v4m_pcie_init(struct rcar_gen4_pcie *rcar)return 0 ; } +static void rcar_gen5_pcie_deinit (struct rcar_gen4_pcie *rcar) +{ +phy_exit (rcar->phy); +rcar_gen4_pcie_common_deinit (rcar); +} +static int rcar_gen5_pcie_init (struct rcar_gen4_pcie *rcar) {struct dw_pcie *dw = &rcar->dw;
@@ -369,21 +384,6 @@ static int rcar_gen5_pcie_init(struct rcar_gen4_pcie *rcar)return ret; } -static void rcar_gen4_pcie_common_deinit (struct rcar_gen4_pcie *rcar) -{ -struct dw_pcie *dw = &rcar->dw; - -reset_control_assert (dw->core_rsts[DW_PCIE_PWR_RST ].rstc); -reset_control_assert (dw->core_rsts[DW_PCIE_CORE_RST ].rstc); -clk_bulk_disable_unprepare (DW_PCIE_NUM_CORE_CLKS , dw->core_clks); -} - -static void rcar_gen5_pcie_deinit (struct rcar_gen4_pcie *rcar) -{ -phy_exit (rcar->phy); -rcar_gen4_pcie_common_deinit (rcar); -} -static int rcar_gen4_pcie_prepare (struct rcar_gen4_pcie *rcar) {struct device *dev = rcar->dw.dev;
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 06/17] PCI: rcar-gen4: Move deinitialization helpers before SoC initialization
Sashiko has reviewed this patch and found no issues.
Quoted 11 lines
Move
common_deinit()next tocommon_init(), ahead ofrcar_gen5_pcie_init(), which will call it when PHY initialization fails. Placercar_gen5_pcie_deinit()beforercar_gen5_pcie_init()as well.Keep these moves separate from the following initialization refactoring to make that patch easier to review. The function bodies are unchanged.
No functional changes.
Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
PATCH v5 7/17 PCI: rcar-gen4: Assert resets when Gen5 SoC PHY initialization fails
In reply to Koichiro Dencommon_init() enables the clocks and deasserts the power and Application/Local resets before phy_init(). If phy_init() fails, the error path only disables the clocks, leaving those resets deasserted.
Assert both resets before disabling the clocks to unwind the completed common initialization.
---
Changes since v3:
- No changes.
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pci/controller/dwc/pcie-rcar-gen4.c +1 -1
@@ -379,7 +379,7 @@ static int rcar_gen5_pcie_init(struct rcar_gen4_pcie *rcar)return 0 ; err_unprepare: -clk_bulk_disable_unprepare (DW_PCIE_NUM_CORE_CLKS , dw->core_clks); +rcar_gen4_pcie_common_deinit (rcar);return ret; }
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 07/17] PCI: rcar-gen4: Assert resets when Gen5 SoC PHY initialization fails
Sashiko has reviewed this patch and found no issues.
Quoted 10 lines
common_init()enables the clocks and deasserts the power and Application/Local resets beforephy_init(). Ifphy_init()fails, the error path only disables the clocks, leaving those resets deasserted.Assert both resets before disabling the clocks to unwind the completed common initialization.
Fixes 3283c4db6009 ("PCI: rcar-gen4: Add support for R-Car X5H PCIe4")Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
PATCH v5 8/17 PCI: rcar-gen4: Separate hardware setup from resource acquisition
In reply to Koichiro DenFactor out the hardware setup needed by Root Port recovery into helpers. Recovery needs to reset and restore the controller while keeping its clocks enabled and the R-Car Gen5 SoC PHY initialized.
- From rcar_gen4_pcie_common_init(), extract the power reset and mode setup. Leave clock enabling and Application/Local reset control in common_init(), so recovery can keep the APP registers accessible.
- From rcar_gen4_v4h_v4m_pcie_init() and rcar_gen5_pcie_init(), extract the SoC-specific register setup. This allows recovery to restore those settings without taking another clock or PHY initialization reference.
- From rcar_gen4_pcie_host_init(), extract the Root Port register setup and PERST# release. Both paths need this host-specific sequence after the per-SoC initialization or reset.
No functional changes.
---
Changes since v3:
- No changes.
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 125 ++++++++++++-------- 1 file changed, 77 insertions(+), 48 deletions(-)
drivers/pci/controller/dwc/pcie-rcar-gen4.c +77 -48
@@ -234,20 +234,13 @@ static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw) rcar->drvdata->ltssm_control (rcar, false); } -static int rcar_gen4_pcie_common_init (struct rcar_gen4_pcie *rcar) +/* Clocks must be enabled and the Application/Local reset deasserted. */ +static int rcar_gen4_pcie_reset (struct rcar_gen4_pcie *rcar) {struct dw_pcie *dw = &rcar->dw;u32 val;int ret; - ret =clk_bulk_prepare_enable (DW_PCIE_NUM_CORE_CLKS , dw->core_clks); -if (ret) { -dev_err (dw->dev,"Enabling core clocks failed\n" ); -return ret; - } - -reset_control_deassert (dw->core_rsts[DW_PCIE_CORE_RST ].rstc); -if (!reset_control_status (dw->core_rsts[DW_PCIE_PWR_RST ].rstc)) {reset_control_assert (dw->core_rsts[DW_PCIE_PWR_RST ].rstc);/*
@@ -266,8 +259,7 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar) }else if (rcar->drvdata->mode ==DW_PCIE_EP_TYPE ) { val |=DEVICE_TYPE_EP ; }else { - ret = -EINVAL ; - goto err_unprepare; +return -EINVAL ; }if (dw->num_lanes <4 )
@@ -277,7 +269,7 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar) ret =reset_control_deassert (dw->core_rsts[DW_PCIE_PWR_RST ].rstc);if (ret) - goto err_unprepare; +return ret;/* * Assure the reset is latched and the core is ready for DBI access.
@@ -293,6 +285,26 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar)fsleep (1000 );return 0 ; +} + +static int rcar_gen4_pcie_common_init (struct rcar_gen4_pcie *rcar) +{ +struct dw_pcie *dw = &rcar->dw; +int ret; + + ret =clk_bulk_prepare_enable (DW_PCIE_NUM_CORE_CLKS , dw->core_clks); +if (ret) { +dev_err (dw->dev,"Enabling core clocks failed\n" ); +return ret; + } + +reset_control_deassert (dw->core_rsts[DW_PCIE_CORE_RST ].rstc); + + ret =rcar_gen4_pcie_reset (rcar); +if (ret) + goto err_unprepare; + +return 0 ; err_unprepare:reset_control_assert (dw->core_rsts[DW_PCIE_CORE_RST ].rstc);
@@ -310,16 +322,10 @@ static void rcar_gen4_pcie_common_deinit(struct rcar_gen4_pcie *rcar)clk_bulk_disable_unprepare (DW_PCIE_NUM_CORE_CLKS , dw->core_clks); } -static int rcar_gen4_v4h_v4m_pcie_init (struct rcar_gen4_pcie *rcar) +static void rcar_gen4_v4h_v4m_pcie_configure (struct rcar_gen4_pcie *rcar) {struct dw_pcie *dw = &rcar->dw;u32 val; -int ret; - -/* R-Car Gen4 common initialization. */ - ret =rcar_gen4_pcie_common_init (rcar); -if (ret) -return ret;/* R-Car V4H and V4M specific additional initialization. */ val =dw_pcie_readl_dbi (dw,PORT_LANE_SKEW );
@@ -331,6 +337,17 @@ static int rcar_gen4_v4h_v4m_pcie_init(struct rcar_gen4_pcie *rcar) val =readl (rcar->base +PCIEPWRMNGCTRL ); val |=APP_CLK_REQ_N |APP_CLK_PM_EN ;writel (val, rcar->base +PCIEPWRMNGCTRL ); +} + +static int rcar_gen4_v4h_v4m_pcie_init (struct rcar_gen4_pcie *rcar) +{ +int ret; + + ret =rcar_gen4_pcie_common_init (rcar); +if (ret) +return ret; + +rcar_gen4_v4h_v4m_pcie_configure (rcar);return 0 ; }
@@ -341,22 +358,11 @@ static void rcar_gen5_pcie_deinit(struct rcar_gen4_pcie *rcar)rcar_gen4_pcie_common_deinit (rcar); } -static int rcar_gen5_pcie_init (struct rcar_gen4_pcie *rcar) +static void rcar_gen5_pcie_configure (struct rcar_gen4_pcie *rcar) {struct dw_pcie *dw = &rcar->dw; -int ret;u32 val; -/* R-Car Gen4 and Gen5 common initialization. */ - ret =rcar_gen4_pcie_common_init (rcar); -if (ret) -return ret; - -/* R-Car Gen5 specific additional initialization. */ - ret =phy_init (rcar->phy); -if (ret) - goto err_unprepare; -dw_pcie_dbi_ro_wr_en (dw); val =dw_pcie_readl_dbi (dw,PORT_LANE_SKEW );
@@ -375,13 +381,25 @@ static int rcar_gen5_pcie_init(struct rcar_gen4_pcie *rcar) val |=APP_CLK_REQ_N |APP_CLK_PM_EN |APP_READY_ENTR_L23 |APP_REQ_ENTR_L1 ;writel (val, rcar->base +PCIEPWRMNGCTRL ); +} -return 0 ; +static int rcar_gen5_pcie_init (struct rcar_gen4_pcie *rcar) +{ +int ret; -err_unprepare: -rcar_gen4_pcie_common_deinit (rcar); + ret =rcar_gen4_pcie_common_init (rcar); +if (ret) +return ret; -return ret; + ret =phy_init (rcar->phy); +if (ret) { +rcar_gen4_pcie_common_deinit (rcar); +return ret; + } + +rcar_gen5_pcie_configure (rcar); + +return 0 ; }static int rcar_gen4_pcie_prepare (struct rcar_gen4_pcie *rcar)
@@ -619,21 +637,11 @@ static void rcar_gen4_pcie_host_perst_assert(struct dw_pcie_rp *pp, bool assert) }/* Host mode */ -static int rcar_gen4_pcie_host_init (struct dw_pcie_rp *pp) +static int rcar_gen4_pcie_host_setup (struct dw_pcie_rp *pp) {struct dw_pcie *dw =to_dw_pcie_from_pp (pp); -struct rcar_gen4_pcie *rcar =to_rcar_gen4_pcie (dw);int ret; -if (pp->bridge) - pp->bridge->enable_device = rcar_gen4_pcie_enable_device; - -rcar_gen4_pcie_host_perst_assert (pp, true); - - ret = rcar->drvdata->init (rcar); -if (ret) -return ret; -/* * According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode * Rev.5.20a and 3.5.6.1 "RC mode" in DWC PCIe RC databook v5.20a, we
@@ -645,13 +653,34 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp) ret =rcar_gen4_pcie_host_msi_init (pp);if (ret) - goto err; +return ret;msleep (PCIE_T_PVPERL_MS );/* pe_rst requires 100msec delay */ -rcar_gen4_pcie_host_perst_assert (pp, false);return 0 ; +} + +static int rcar_gen4_pcie_host_init (struct dw_pcie_rp *pp) +{ +struct dw_pcie *dw =to_dw_pcie_from_pp (pp); +struct rcar_gen4_pcie *rcar =to_rcar_gen4_pcie (dw); +int ret; + +if (pp->bridge) + pp->bridge->enable_device = rcar_gen4_pcie_enable_device; + +rcar_gen4_pcie_host_perst_assert (pp, true); + + ret = rcar->drvdata->init (rcar); +if (ret) +return ret; + + ret =rcar_gen4_pcie_host_setup (pp); +if (ret) + goto err; + +return 0 ; err: rcar->drvdata->deinit (rcar);
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 08/17] PCI: rcar-gen4: Separate hardware setup from resource acquisition
Sashiko has reviewed this patch and found no issues.
Quoted 12 lines
Factor out the hardware setup needed by Root Port recovery into helpers. Recovery needs to reset and restore the controller while keeping its clocks enabled and the R-Car Gen5 SoC PHY initialized.
- From
rcar_gen4_pcie_common_init(), extract the power reset and mode setup. Leave clock enabling and Application/Local reset control incommon_init(), so recovery can keep the APP registers accessible.- From
rcar_gen4_v4h_v4m_pcie_init()andrcar_gen5_pcie_init(), extract the SoC-specific register setup. This allows recovery to restore those settings without taking another clock or PHY initialization reference.
Quoted 8 lines
- From
rcar_gen4_pcie_host_init(), extract the Root Port register setup and PERST# release. Both paths need this host-specific sequence after the per-SoC initialization or reset.No functional changes.
Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
PATCH v5 9/17 PCI: rcar-gen4: Add Root Port reset support
In reply to Koichiro DenImplement the host bridge reset_root_port() callback so PCI error recovery can reset and reinitialize the R-Car controller. This also provides the reset operation for the link-down handling added later.
Reinitialize without going through .deinit() and .init(), which would release and re-acquire the clocks and the R-Car Gen5 SoC PHY: split the SoC-specific register setup that the power reset wipes out into a .configure callback and repeat only that, then restore the Root Port registers and restart link training.
Rather than tracking which APP interrupt enables survive the power reset, derive them from software state through a single helper. A flag keeps the sources masked from the start of a reset until one succeeds, so a failed reinitialization does not re-enable them against an uninitialized controller.
Reject a reset while the controller is suspended, as a failed resume may have left it powered down.
Track whether the R-Car Gen5 SoC PHY power-on has been requested so a reset retry, or teardown after a failed reset, does not call phy_power_off() twice. Continue to ignore the PHY power operation return values.
---
Changes since v4:
- Use scoped_guard() in the MSI setup error path. (Marek)
https://lore.kernel.org/r/[email protected]/
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 146 ++++++++++++++++++-- 1 file changed, 136 insertions(+), 10 deletions(-)
drivers/pci/controller/dwc/pcie-rcar-gen4.c +136 -10
@@ -99,6 +99,8 @@ struct rcar_gen4_pcie;struct rcar_gen4_pcie_drvdata {int (*init)(struct rcar_gen4_pcie *rcar);void (*deinit)(struct rcar_gen4_pcie *rcar); +/* SoC-specific register setup, repeated after a power reset */ +void (*configure)(struct rcar_gen4_pcie *rcar);int (*ltssm_control)(struct rcar_gen4_pcie *rcar,bool enable);int (*speed_control)(struct rcar_gen4_pcie *rcar);enum dw_pcie_device_mode mode;
@@ -109,9 +111,17 @@ struct rcar_gen4_pcie {void __iomem *base;void __iomem *phy_base;struct phy *phy; +bool phy_powered;struct platform_device *pdev;struct reset_control *perst;const struct rcar_gen4_pcie_drvdata *drvdata; +/* +* The controller needs reinitializing; the APP interrupt sources +* stay masked until a reset succeeds. +*/ +bool reinit_pending; +/* Protects APP interrupt enable registers and their software state. */ +raw_spinlock_t app_lock; };#define to_rcar_gen4_pcie (_dw) container_of(_dw, struct rcar_gen4_pcie, dw)
@@ -322,6 +332,21 @@ static void rcar_gen4_pcie_common_deinit(struct rcar_gen4_pcie *rcar)clk_bulk_disable_unprepare (DW_PCIE_NUM_CORE_CLKS , dw->core_clks); } +/* Reset and restore registers while clocks and PHY remain initialized. */ +static int rcar_gen4_pcie_common_reinit (struct rcar_gen4_pcie *rcar) +{ +int ret; + + ret =rcar_gen4_pcie_reset (rcar); +if (ret) +return ret; + +if (rcar->drvdata->configure) + rcar->drvdata->configure (rcar); + +return 0 ; +} +static void rcar_gen4_v4h_v4m_pcie_configure (struct rcar_gen4_pcie *rcar) {struct dw_pcie *dw = &rcar->dw;
@@ -513,6 +538,26 @@ static int rcar_gen4_pcie_host_msi_addr(struct dw_pcie_rp *pp, u32 *msi_addr)return 0 ; } +static void rcar_gen4_pcie_app_irq_sync_locked (struct rcar_gen4_pcie *rcar) +{ +u32 val; + +lockdep_assert_held (&rcar->app_lock); + + val =readl (rcar->base +PCIEINTSTS0EN ); + val &= ~MSI_CTRL_INT ; +if (!rcar->reinit_pending &&pci_msi_enabled ()) + val |=MSI_CTRL_INT ; +writel (val, rcar->base +PCIEINTSTS0EN ); +} + +static void rcar_gen4_pcie_app_irq_sync (struct rcar_gen4_pcie *rcar) +{ +guard (raw_spinlock_irqsave)(&rcar->app_lock); + +rcar_gen4_pcie_app_irq_sync_locked (rcar); +} +static int rcar_gen4_pcie_host_msi_init (struct dw_pcie_rp *pp) {struct dw_pcie *dw =to_dw_pcie_from_pp (pp);
@@ -543,12 +588,7 @@ static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp) }/* Configure MSI interrupt signal */ - val =readl (rcar->base +PCIEINTSTS0EN ); -if (pci_msi_enabled ()) - val |=MSI_CTRL_INT ; -else - val &= ~MSI_CTRL_INT ; -writel (val, rcar->base +PCIEINTSTS0EN ); +rcar_gen4_pcie_app_irq_sync (rcar);return 0 ;
@@ -563,9 +603,11 @@ static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)writel (0 , rcar->base +AXIINTCCONT );/* Deconfigure MSI interrupt signal */ - val =readl (rcar->base +PCIEINTSTS0EN ); - val &= ~MSI_CTRL_INT ; -writel (val, rcar->base +PCIEINTSTS0EN ); + scoped_guard(raw_spinlock_irqsave, &rcar->app_lock) { + val =readl (rcar->base +PCIEINTSTS0EN ); + val &= ~MSI_CTRL_INT ; +writel (val, rcar->base +PCIEINTSTS0EN ); + }return ret; }
@@ -661,14 +703,87 @@ static int rcar_gen4_pcie_host_setup(struct dw_pcie_rp *pp)return 0 ; } +static void rcar_gen4_pcie_quiesce_irqs (struct rcar_gen4_pcie *rcar) +{ + scoped_guard(raw_spinlock_irqsave, &rcar->app_lock) { + rcar->reinit_pending = true; +rcar_gen4_pcie_app_irq_sync_locked (rcar); + } +} + +static void rcar_gen4_pcie_resume_irqs (struct rcar_gen4_pcie *rcar, +bool recovered) +{ + scoped_guard(raw_spinlock_irqsave, &rcar->app_lock) { + rcar->reinit_pending = !recovered; +rcar_gen4_pcie_app_irq_sync_locked (rcar); + } +} + +/* +* R-Car Gen4 SoC PCIe controllers and R-Car Gen5 SoC PCIe4 controller have +* a single Root Port per instance, so the 'pci_dev' is ignored and the whole +* controller is reset. The PCI core serializes the callers through its +* device locks. +*/ +static int rcar_gen4_pcie_reset_root_port (struct pci_host_bridge *bridge, +struct pci_dev *pdev) +{ +struct rcar_gen4_pcie *rcar =dev_get_drvdata (bridge->dev.parent); +struct dw_pcie *dw = &rcar->dw; +struct dw_pcie_rp *pp = &dw->pp; +struct device *dev = dw->dev; +int ret; + +/* A failed resume may have left the controller powered down. */ +if (dw->suspended) +return -EBUSY ; + +rcar_gen4_pcie_quiesce_irqs (rcar); + +dw_pcie_stop_link (dw); + +rcar_gen4_pcie_host_perst_assert (pp, true); + + ret =rcar_gen4_pcie_common_reinit (rcar); +if (ret) + goto out; + + ret =rcar_gen4_pcie_host_setup (pp); +if (ret) + goto out; + + ret =dw_pcie_setup_rc (pp); +if (ret) + goto out; + + ret =dw_pcie_start_link (dw); +if (ret) + goto out; + +/* Ignore errors, the link may come up later */ +dw_pcie_wait_for_link (dw); +dev_dbg (dev,"Root Port reset completed\n" ); + +out: +rcar_gen4_pcie_resume_irqs (rcar, !ret); +if (ret) +dev_err (dev,"Root Port reset failed (%d); interrupts stay masked until a reset succeeds\n" , + ret); + +return ret; +} +static int rcar_gen4_pcie_host_init (struct dw_pcie_rp *pp) {struct dw_pcie *dw =to_dw_pcie_from_pp (pp);struct rcar_gen4_pcie *rcar =to_rcar_gen4_pcie (dw);int ret; -if (pp->bridge) +if (pp->bridge) { pp->bridge->enable_device = rcar_gen4_pcie_enable_device; + pp->bridge->reset_root_port = rcar_gen4_pcie_reset_root_port; + }rcar_gen4_pcie_host_perst_assert (pp, true);
@@ -708,6 +823,8 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)if (!IS_ENABLED (CONFIG_PCIE_RCAR_GEN4_HOST ))return -ENODEV ; +raw_spin_lock_init (&rcar->app_lock); + pp->num_vectors =MAX_MSI_IRQS ;/* Reserve an iATU window for the generic PME_Turn_Off implementation. */ pp->use_atu_msg = true;
@@ -944,11 +1061,17 @@ static int rcar_gen5_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable }writel (val, rcar->base +PCIERSTCTRL1 ); +/* A reset retry may find the PHY already powered off. */ +if (enable == rcar->phy_powered) +return 0 ; +if (enable)phy_power_on (rcar->phy);else phy_power_off (rcar->phy); + rcar->phy_powered = enable; +return 0 ; }
@@ -1141,6 +1264,7 @@ static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie_ep = {static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie = { .init = rcar_gen4_v4h_v4m_pcie_init, .deinit = rcar_gen4_pcie_common_deinit, + .configure = rcar_gen4_v4h_v4m_pcie_configure, .ltssm_control = rcar_gen4_pcie_ltssm_control, .speed_control = rcar_gen4_pcie_speed_control, .mode =DW_PCIE_RC_TYPE ,
@@ -1149,6 +1273,7 @@ static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie = {static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie_ep = { .init = rcar_gen4_v4h_v4m_pcie_init, .deinit = rcar_gen4_pcie_common_deinit, + .configure = rcar_gen4_v4h_v4m_pcie_configure, .ltssm_control = rcar_gen4_pcie_ltssm_control, .speed_control = rcar_gen4_pcie_speed_control, .mode =DW_PCIE_EP_TYPE ,
@@ -1157,6 +1282,7 @@ static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie_ep = {static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen5_pcie = { .init = rcar_gen5_pcie_init, .deinit = rcar_gen5_pcie_deinit, + .configure = rcar_gen5_pcie_configure, .ltssm_control = rcar_gen5_pcie_ltssm_control, .speed_control = rcar_gen5_pcie_speed_control, .mode =DW_PCIE_RC_TYPE ,
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 09/17] PCI: rcar-gen4: Add Root Port reset support
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] PCI: rcar-gen4: missing ltssm_control() unwind in rcar_gen4_pcie_start_link() --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] PCI: rcar-gen4: missing ltssm_control() unwind in rcar_gen4_pcie_start_link() --
commit d7293bc34efbc4c17b8f7975af710d02c706b7d0
Author: Koichiro Den <[email protected]>
PCI: rcar-gen4: Add Root Port reset support
This commit implements the host bridge reset_root_port() callback to reset and reinitialize the R-Car PCIe controller without releasing clocks or the R-Car Gen5 SoC PHY. It also synchronizes APP interrupt enables and tracks the R-Car Gen5 PHY power state across resets.
Quoted 4 lines
drivers/pci/controller/dwc/pcie-rcar-gen4.c +0
-0
[ ... ]
Quoted 74 lines
drivers/pci/controller/dwc/pcie-rcar-gen4.c +70
-0@@ -661,14 +703,87 @@ static int rcar_gen4_pcie_host_setup(struct dw_pcie_rp *pp)return 0 ; } +static void rcar_gen4_pcie_quiesce_irqs (struct rcar_gen4_pcie *rcar) +{ + scoped_guard(raw_spinlock_irqsave, &rcar->app_lock) { + rcar->reinit_pending = true; +rcar_gen4_pcie_app_irq_sync_locked (rcar); + } +} + +static void rcar_gen4_pcie_resume_irqs (struct rcar_gen4_pcie *rcar, +bool recovered) +{ + scoped_guard(raw_spinlock_irqsave, &rcar->app_lock) { + rcar->reinit_pending = !recovered; +rcar_gen4_pcie_app_irq_sync_locked (rcar); + } +} + +/* +* R-Car Gen4 SoC PCIe controllers and R-Car Gen5 SoC PCIe4 controller have +* a single Root Port per instance, so the 'pci_dev' is ignored and the whole +* controller is reset. The PCI core serializes the callers through its +* device locks. +*/ +static int rcar_gen4_pcie_reset_root_port (struct pci_host_bridge *bridge, +struct pci_dev *pdev) +{ +struct rcar_gen4_pcie *rcar =dev_get_drvdata (bridge->dev.parent); +struct dw_pcie *dw = &rcar->dw; +struct dw_pcie_rp *pp = &dw->pp; +struct device *dev = dw->dev; +int ret; + +/* A failed resume may have left the controller powered down. */ +if (dw->suspended) +return -EBUSY ; + +rcar_gen4_pcie_quiesce_irqs (rcar); + +dw_pcie_stop_link (dw); + +rcar_gen4_pcie_host_perst_assert (pp, true); + + ret =rcar_gen4_pcie_common_reinit (rcar); +if (ret) + goto out; + + ret =rcar_gen4_pcie_host_setup (pp); +if (ret) + goto out; + + ret =dw_pcie_setup_rc (pp); +if (ret) + goto out; + + ret =dw_pcie_start_link (dw); +if (ret) + goto out; + +/* Ignore errors, the link may come up later */ +dw_pcie_wait_for_link (dw); +dev_dbg (dev,"Root Port reset completed\n" ); + +out: +rcar_gen4_pcie_resume_irqs (rcar, !ret); +if (ret) +dev_err (dev,"Root Port reset failed (%d); interrupts stay masked until a reset succeeds\n" , + ret); + +return ret; +}
[ ... ]
Quoted 7 lines
drivers/pci/controller/dwc/pcie-rcar-gen4.c +3
-0@@ -944,11 +1061,17 @@ static int rcar_gen5_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable }writel (val, rcar->base +PCIERSTCTRL1 ); +/* A reset retry may find the PHY already powered off. */ +if (enable == rcar->phy_powered) +return 0 ;
drivers/pci/controller/dwc/pcie-rcar-gen4.c static int rcar_gen5_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable · line 1076
Quoted 10 lines
drivers/pci/controller/dwc/pcie-rcar-gen4.c +3
-0+if (enable)phy_power_on (rcar->phy);else phy_power_off (rcar->phy); + rcar->phy_powered = enable; +return 0 ; }
[Severity: Medium]
On R-Car Gen5, dw_pcie_start_link() calls rcar_gen4_pcie_start_link(), which enables LTSSM and powers on the PHY before running speed_control():
drivers/pci/controller/dwc/pcie-rcar-gen4.c:rcar_gen4_pcie_start_link() {
...
if (rcar->drvdata->ltssm_control) {
ret = rcar->drvdata->ltssm_control(rcar, true);
if (ret)
return ret;
}
return rcar->drvdata->speed_control(rcar);
}If rcar_gen5_pcie_speed_control() times out and returns -ETIMEDOUT, rcar_gen5_pcie_ltssm_control(rcar, true) has already enabled LTSSM, called phy_power_on(rcar->phy), and set rcar->phy_powered = true.
In rcar_gen4_pcie_reset_root_port(), this failure jumps to out: and leaves rcar->reinit_pending set to true (keeping APP interrupts masked) while PERST# is deasserted, LTSSM remains enabled, and the PHY remains powered on.
Similarly, if dw_pcie_start_link() fails during dw_pcie_resume_noirq() or dw_pcie_host_init(), those callers bypass their dw_pcie_stop_link() error label and call rcar_gen5_pcie_deinit() -> phy_exit(rcar->phy) while the PHY is still powered on and rcar->phy_powered is still true. On a subsequent resume, rcar_gen5_pcie_ltssm_control(rcar, true) will hit the (enable == rcar->phy_powered) check and skip phy_power_on().
Should rcar_gen4_pcie_start_link() unwind with rcar->drvdata->ltssm_control(rcar, false) when speed_control() fails?
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
PATCH v5 10/17 PCI: dwc: Free the MSI domain after the host .deinit() callback
In reply to Koichiro Dendw_pcie_host_deinit() frees the iMSI-RX domain before calling the host .deinit() callback. For a core-owned parent interrupt, dw_pcie_free_msi() removes the chained handler right before the domain. A glue driver that owns the parent interrupt and demultiplexes the MSIs itself through dw_handle_msi_irq() can only stop its handler in .deinit(), after the domain is gone, so a late interrupt can still dispatch into the freed domain.
Remove the domain after .deinit(), in dw_pcie_host_deinit() and in the dw_pcie_host_init() unwind. Keep removing the chained handlers of core-owned interrupts before .deinit(), which powers the controller down. Clear pp->irq_domain when the domain is removed so the unwind can tell whether it exists.
---
Changes since v4:
- Add Marek's Reviewed-by tag. No code changes.
.../pci/controller/dwc/pcie-designware-host.c | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-)
drivers/pci/controller/dwc/pcie-designware-host.c +23 -8
@@ -228,7 +228,7 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp) }EXPORT_SYMBOL_GPL (dw_pcie_allocate_domains); -void dw_pcie_free_msi (struct dw_pcie_rp *pp) +static void dw_pcie_remove_msi_handlers (struct dw_pcie_rp *pp) {u32 ctrl;
@@ -236,8 +236,18 @@ void dw_pcie_free_msi(struct dw_pcie_rp *pp)if (pp->msi_irq[ctrl] >0 )irq_set_chained_handler_and_data (pp->msi_irq[ctrl],NULL ,NULL ); } +} +static void dw_pcie_free_msi_domain (struct dw_pcie_rp *pp) +{irq_domain_remove (pp->irq_domain); + pp->irq_domain =NULL ; +} + +void dw_pcie_free_msi (struct dw_pcie_rp *pp) +{ +dw_pcie_remove_msi_handlers (pp); +dw_pcie_free_msi_domain (pp); }EXPORT_SYMBOL_GPL (dw_pcie_free_msi);
@@ -633,7 +643,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =of_pci_get_equalization_presets (dev, &pp->presets, pci->num_lanes);if (ret) - goto err_free_msi; + goto err_deinit_host;/* * Allocate the resource for MSG TLP before programming the iATU
@@ -649,7 +659,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =dw_pcie_edma_detect (pci);if (ret) - goto err_free_msi; + goto err_deinit_host; ret =dw_pcie_setup_rc (pp);if (ret)
@@ -686,14 +696,16 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) err_remove_edma:dw_pcie_edma_remove (pci); -err_free_msi: -if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); - err_deinit_host: +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_remove_msi_handlers (pp); +if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_free_msi_domain (pp); + err_free_ecam:if (pp->cfg)pci_ecam_free (pp->cfg);
@@ -718,11 +730,14 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)dw_pcie_edma_remove (pci);if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); +dw_pcie_remove_msi_handlers (pp);if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx) +dw_pcie_free_msi_domain (pp); +if (pp->cfg)pci_ecam_free (pp->cfg); }
Signature · 1 line
2.54.0
Re: [PATCH v5 10/17] PCI: dwc: Free the MSI domain after the host .deinit() callback
Hi Koichiro
Quoted 8 lines · Koichiro Den
dw_pcie_host_deinit()frees the iMSI-RX domain before calling the host .deinit() callback. For a core-owned parent interrupt,dw_pcie_free_msi()removes the chained handler right before the domain. A glue driver that owns the parent interrupt and demultiplexes the MSIs itself throughdw_handle_msi_irq()can only stop its handler in .deinit(), after the domain is gone, so a late interrupt can still dispatch into the freed domain.
I think you need a rebase anyway since there are two patches touching MSI irq naming stuff in pci-tree's next branch and will probably show up in tommorow's linux-next.
...
drivers/pci/controller/dwc/pcie-designware-host.c void dw_pcie_free_msi(struct dw_pcie_rp *pp) · line 252
Quoted 13 lines
drivers/pci/controller/dwc/pcie-designware-host.c +9
-0+static void dw_pcie_free_msi_domain (struct dw_pcie_rp *pp) +{irq_domain_remove (pp->irq_domain); + pp->irq_domain =NULL ; +} + +void dw_pcie_free_msi (struct dw_pcie_rp *pp) +{ +dw_pcie_remove_msi_handlers (pp); +dw_pcie_free_msi_domain (pp); }EXPORT_SYMBOL_GPL (dw_pcie_free_msi);
dw_pcie_free_msi() now calls freeing irq_domain unconditionally:
The problem is that pcie-qcom.c uses the ECAM init path, which does not go through dw_pcie_host_init(), and registers its own devres action, see:drivers/pci/controller/dwc/pcie-qcom.c:2034
return devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
drivers/pci/controller/dwc/pcie-qcom.c:1995
static void qcom_pci_free_msi(void *ptr)
{
struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
if (pp && pp->use_imsi_rx)
dw_pcie_free_msi(pp);
}This leads to the following sequence:
1. dw_pcie_host_init() succeeds and pp->irq_domain is created;
2. the driver is removed or probe fails, dw_pcie_host_deinit() calls
dw_pcie_free_msi_domain() and sets pp->irq_domain to NULL;
3. the devres action qcom_pci_free_msi() still holds the same pp and calls dw_pcie_free_msi(), which enters dw_pcie_free_msi_domain() again;
4. irq_domain_remove(NULL) dereferences a NULL pointer and the kernel crashes.
So I suggest you to add a safe check if (!pp->irq_domain) before doing that.
Quoted 55 lines
drivers/pci/controller/dwc/pcie-designware-host.c +12
-7@@ -633,7 +643,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =of_pci_get_equalization_presets (dev, &pp->presets, pci->num_lanes);if (ret) - goto err_free_msi; + goto err_deinit_host;/* * Allocate the resource for MSG TLP before programming the iATU @@ -649,7 +659,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =dw_pcie_edma_detect (pci);if (ret) - goto err_free_msi; + goto err_deinit_host; ret =dw_pcie_setup_rc (pp);if (ret)@@ -686,14 +696,16 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) err_remove_edma:dw_pcie_edma_remove (pci); -err_free_msi: -if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); - err_deinit_host: +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_remove_msi_handlers (pp); +if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_free_msi_domain (pp); + err_free_ecam:if (pp->cfg)pci_ecam_free (pp->cfg);@@ -718,11 +730,14 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)dw_pcie_edma_remove (pci);if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); +dw_pcie_remove_msi_handlers (pp);if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx) +dw_pcie_free_msi_domain (pp); +if (pp->cfg)pci_ecam_free (pp->cfg); }
Re: [PATCH v5 10/17] PCI: dwc: Free the MSI domain after the host .deinit() callback
Quoted 12 lines · Shawn Lin
Hi Koichiro
Quoted 8 lines · Koichiro Den
dw_pcie_host_deinit()frees the iMSI-RX domain before calling the host .deinit() callback. For a core-owned parent interrupt,dw_pcie_free_msi()removes the chained handler right before the domain. A glue driver that owns the parent interrupt and demultiplexes the MSIs itself throughdw_handle_msi_irq()can only stop its handler in .deinit(), after the domain is gone, so a late interrupt can still dispatch into the freed domain.
Quoted 3 lines
I think you need a rebase anyway since there are two patches touching MSI irq naming stuff in pci-tree's next branch and will probably show up in tommorow's linux-next.
Hi Shawn, thanks for the heads-up! And thanks for the review.
Quoted 34 lines
...
drivers/pci/controller/dwc/pcie-designware-host.c void dw_pcie_free_msi(struct dw_pcie_rp *pp) · line 252
Quoted 12 lines
drivers/pci/controller/dwc/pcie-designware-host.c +9
-0+static void dw_pcie_free_msi_domain (struct dw_pcie_rp *pp) +{irq_domain_remove (pp->irq_domain); + pp->irq_domain =NULL ; +} + +void dw_pcie_free_msi (struct dw_pcie_rp *pp) +{ +dw_pcie_remove_msi_handlers (pp); +dw_pcie_free_msi_domain (pp); }EXPORT_SYMBOL_GPL (dw_pcie_free_msi);
dw_pcie_free_msi()now calls freeingirq_domainunconditionally:The problem is that
pcie-qcom.cuses the ECAM init path, which does not go throughdw_pcie_host_init(), and registers its own devres action, see:drivers/pci/controller/dwc/pcie-qcom.c:2034return devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
drivers/pci/controller/dwc/pcie-qcom.c:1995static void qcom_pci_free_msi(void *ptr) { struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr; if (pp && pp->use_imsi_rx) dw_pcie_free_msi(pp); }
Quoted 8 lines
This leads to the following sequence:
1.
dw_pcie_host_init()succeeds andpp->irq_domainis created;
2. the driver is removed or probe fails,dw_pcie_host_deinit()callsdw_pcie_free_msi_domain() and sets pp->irq_domain to NULL;3. the devres action
qcom_pci_free_msi()still holds the same pp and callsdw_pcie_free_msi(), which entersdw_pcie_free_msi_domain()again;
I'm a bit confused. Steps 1 and 2 here seem to describe the regular DWC path, but the devres action in step 3 looks like it's only registered in the firmware-managed ECAM path. I don't see where that path would call dw_pcie_host_deinit(). Am I missing something?
Best regards,
Koichiro Den
Quoted 55 lines
4.
irq_domain_remove(NULL)dereferences a NULL pointer and the kernel crashes.So I suggest you to add a safe check if (!pp->irq_domain) before doing that.
Quoted 46 lines
drivers/pci/controller/dwc/pcie-designware-host.c +12
-7@@ -633,7 +643,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =of_pci_get_equalization_presets (dev, &pp->presets, pci->num_lanes);if (ret) - goto err_free_msi; + goto err_deinit_host;/* * Allocate the resource for MSG TLP before programming the iATU @@ -649,7 +659,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =dw_pcie_edma_detect (pci);if (ret) - goto err_free_msi; + goto err_deinit_host; ret =dw_pcie_setup_rc (pp);if (ret)@@ -686,14 +696,16 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) err_remove_edma:dw_pcie_edma_remove (pci); -err_free_msi: -if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); - err_deinit_host: +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_remove_msi_handlers (pp); +if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_free_msi_domain (pp); + err_free_ecam:if (pp->cfg)pci_ecam_free (pp->cfg);@@ -718,11 +730,14 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)dw_pcie_edma_remove (pci);if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); +dw_pcie_remove_msi_handlers (pp);if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx) +dw_pcie_free_msi_domain (pp); +if (pp->cfg)pci_ecam_free (pp->cfg); }
Re: [PATCH v5 10/17] PCI: dwc: Free the MSI domain after the host .deinit() callback
Quoted 62 lines · Koichiro Den
Quoted 12 lines · Shawn Lin
Hi Koichiro
Quoted 8 lines · Koichiro Den
dw_pcie_host_deinit()frees the iMSI-RX domain before calling the host .deinit() callback. For a core-owned parent interrupt,dw_pcie_free_msi()removes the chained handler right before the domain. A glue driver that owns the parent interrupt and demultiplexes the MSIs itself throughdw_handle_msi_irq()can only stop its handler in .deinit(), after the domain is gone, so a late interrupt can still dispatch into the freed domain.Quoted 3 lines
I think you need a rebase anyway since there are two patches touching MSI irq naming stuff in pci-tree's next branch and will probably show up in tommorow's linux-next.
Hi Shawn, thanks for the heads-up! And thanks for the review.
Quoted 34 lines
...
drivers/pci/controller/dwc/pcie-designware-host.c void dw_pcie_free_msi(struct dw_pcie_rp *pp) · line 252
Quoted 12 lines
drivers/pci/controller/dwc/pcie-designware-host.c +9
-0+static void dw_pcie_free_msi_domain (struct dw_pcie_rp *pp) +{irq_domain_remove (pp->irq_domain); + pp->irq_domain =NULL ; +} + +void dw_pcie_free_msi (struct dw_pcie_rp *pp) +{ +dw_pcie_remove_msi_handlers (pp); +dw_pcie_free_msi_domain (pp); }EXPORT_SYMBOL_GPL (dw_pcie_free_msi);
dw_pcie_free_msi()now calls freeingirq_domainunconditionally:The problem is that
pcie-qcom.cuses the ECAM init path, which does not go throughdw_pcie_host_init(), and registers its own devres action, see:drivers/pci/controller/dwc/pcie-qcom.c:2034return devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
drivers/pci/controller/dwc/pcie-qcom.c:1995static void qcom_pci_free_msi(void *ptr) { struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr; if (pp && pp->use_imsi_rx) dw_pcie_free_msi(pp); }Quoted 8 lines
This leads to the following sequence:
1.
dw_pcie_host_init()succeeds andpp->irq_domainis created;
2. the driver is removed or probe fails,dw_pcie_host_deinit()callsdw_pcie_free_msi_domain() and sets pp->irq_domain to NULL;3. the devres action
qcom_pci_free_msi()still holds the same pp and callsdw_pcie_free_msi(), which entersdw_pcie_free_msi_domain()again;
Quoted 5 lines
I'm a bit confused. Steps 1 and 2 here seem to describe the regular DWC path, but the devres action in step 3 looks like it's only registered in the firmware-managed ECAM path. I don't see where that path would call
dw_pcie_host_deinit(). Am I missing something?
My apologies for any confusion my earlier response may have caused. I brought up qcom merely as an example of a driver managing the MSI IRQ lifecycle with its own devres action, with a remove path that can go through dw_pcie_host_deinit(). For such combinations the safe check I suggested would matter, I intended this as a defensive improvement, not necessarily implying a bug exists now.
Quoted 59 lines
Best regards,
Koichiro DenQuoted 55 lines
4.
irq_domain_remove(NULL)dereferences a NULL pointer and the kernel crashes.So I suggest you to add a safe check if (!pp->irq_domain) before doing that.
Quoted 46 lines
drivers/pci/controller/dwc/pcie-designware-host.c +12
-7@@ -633,7 +643,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =of_pci_get_equalization_presets (dev, &pp->presets, pci->num_lanes);if (ret) - goto err_free_msi; + goto err_deinit_host;/* * Allocate the resource for MSG TLP before programming the iATU @@ -649,7 +659,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =dw_pcie_edma_detect (pci);if (ret) - goto err_free_msi; + goto err_deinit_host; ret =dw_pcie_setup_rc (pp);if (ret)@@ -686,14 +696,16 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) err_remove_edma:dw_pcie_edma_remove (pci); -err_free_msi: -if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); - err_deinit_host: +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_remove_msi_handlers (pp); +if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_free_msi_domain (pp); + err_free_ecam:if (pp->cfg)pci_ecam_free (pp->cfg);@@ -718,11 +730,14 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)dw_pcie_edma_remove (pci);if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); +dw_pcie_remove_msi_handlers (pp);if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx) +dw_pcie_free_msi_domain (pp); +if (pp->cfg)pci_ecam_free (pp->cfg); }
Re: [PATCH v5 10/17] PCI: dwc: Free the MSI domain after the host .deinit() callback
Quoted 69 lines · Shawn Lin
Quoted 62 lines · Koichiro Den
Quoted 12 lines · Shawn Lin
Hi Koichiro
Quoted 8 lines · Koichiro Den
dw_pcie_host_deinit()frees the iMSI-RX domain before calling the host .deinit() callback. For a core-owned parent interrupt,dw_pcie_free_msi()removes the chained handler right before the domain. A glue driver that owns the parent interrupt and demultiplexes the MSIs itself throughdw_handle_msi_irq()can only stop its handler in .deinit(), after the domain is gone, so a late interrupt can still dispatch into the freed domain.Quoted 3 lines
I think you need a rebase anyway since there are two patches touching MSI irq naming stuff in pci-tree's next branch and will probably show up in tommorow's linux-next.
Hi Shawn, thanks for the heads-up! And thanks for the review.
Quoted 34 lines
...
drivers/pci/controller/dwc/pcie-designware-host.c void dw_pcie_free_msi(struct dw_pcie_rp *pp) · line 252
Quoted 12 lines
drivers/pci/controller/dwc/pcie-designware-host.c +9
-0+static void dw_pcie_free_msi_domain (struct dw_pcie_rp *pp) +{irq_domain_remove (pp->irq_domain); + pp->irq_domain =NULL ; +} + +void dw_pcie_free_msi (struct dw_pcie_rp *pp) +{ +dw_pcie_remove_msi_handlers (pp); +dw_pcie_free_msi_domain (pp); }EXPORT_SYMBOL_GPL (dw_pcie_free_msi);
dw_pcie_free_msi()now calls freeingirq_domainunconditionally:The problem is that
pcie-qcom.cuses the ECAM init path, which does not go throughdw_pcie_host_init(), and registers its own devres action, see:drivers/pci/controller/dwc/pcie-qcom.c:2034return devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
drivers/pci/controller/dwc/pcie-qcom.c:1995static void qcom_pci_free_msi(void *ptr) { struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr; if (pp && pp->use_imsi_rx) dw_pcie_free_msi(pp); }Quoted 8 lines
This leads to the following sequence:
1.
dw_pcie_host_init()succeeds andpp->irq_domainis created;
2. the driver is removed or probe fails,dw_pcie_host_deinit()callsdw_pcie_free_msi_domain() and sets pp->irq_domain to NULL;3. the devres action
qcom_pci_free_msi()still holds the same pp and callsdw_pcie_free_msi(), which entersdw_pcie_free_msi_domain()again;Quoted 5 lines
I'm a bit confused. Steps 1 and 2 here seem to describe the regular DWC path, but the devres action in step 3 looks like it's only registered in the firmware-managed ECAM path. I don't see where that path would call
dw_pcie_host_deinit(). Am I missing something?
Quoted 6 lines
My apologies for any confusion my earlier response may have caused. I brought up qcom merely as an example of a driver managing the MSI IRQ lifecycle with its own devres action, with a remove path that can go through
dw_pcie_host_deinit(). For such combinations the safe check I suggested would matter, I intended this as a defensive improvement, not necessarily implying a bug exists now.
Oh I see. I personally agree, and it's quite a cheap check, but I'm not sure what the maintainers of this file think about defensive programming here.
+Cc: Mani
Mani, I'd appreciate your thoughts on this.
Thanks for the review!
Best regards,
Koichiro Den
Quoted 61 lines
Quoted 60 lines
Best regards,
Koichiro DenQuoted 55 lines
4.
irq_domain_remove(NULL)dereferences a NULL pointer and the kernel crashes.So I suggest you to add a safe check if (!pp->irq_domain) before doing that.
Quoted 46 lines
drivers/pci/controller/dwc/pcie-designware-host.c +12
-7@@ -633,7 +643,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =of_pci_get_equalization_presets (dev, &pp->presets, pci->num_lanes);if (ret) - goto err_free_msi; + goto err_deinit_host;/* * Allocate the resource for MSG TLP before programming the iATU @@ -649,7 +659,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) ret =dw_pcie_edma_detect (pci);if (ret) - goto err_free_msi; + goto err_deinit_host; ret =dw_pcie_setup_rc (pp);if (ret)@@ -686,14 +696,16 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) err_remove_edma:dw_pcie_edma_remove (pci); -err_free_msi: -if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); - err_deinit_host: +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_remove_msi_handlers (pp); +if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx && pp->irq_domain) +dw_pcie_free_msi_domain (pp); + err_free_ecam:if (pp->cfg)pci_ecam_free (pp->cfg);@@ -718,11 +730,14 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)dw_pcie_edma_remove (pci);if (pp->use_imsi_rx) -dw_pcie_free_msi (pp); +dw_pcie_remove_msi_handlers (pp);if (pp->ops->deinit) pp->ops->deinit (pp); +if (pp->use_imsi_rx) +dw_pcie_free_msi_domain (pp); +if (pp->cfg)pci_ecam_free (pp->cfg); }
[email protected]
AI reviewRe: [PATCH v5 10/17] PCI: dwc: Free the MSI domain after the host .deinit() callback
In reply to Koichiro DenSashiko has reviewed this patch and found no issues.
Quoted 8 lines
dw_pcie_host_deinit()frees the iMSI-RX domain before calling the host .deinit() callback. For a core-owned parent interrupt,dw_pcie_free_msi()removes the chained handler right before the domain. A glue driver that owns the parent interrupt and demultiplexes the MSIs itself throughdw_handle_msi_irq()can only stop its handler in .deinit(), after the domain is gone, so a late interrupt can still dispatch into the freed domain.
Quoted 8 lines
Remove the domain after .deinit(), in
dw_pcie_host_deinit()and in thedw_pcie_host_init()unwind. Keep removing the chained handlers of core-owned interrupts before .deinit(), which powers the controller down. Clearpp->irq_domainwhen the domain is removed so the unwind can tell whether it exists.Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10
Re: [PATCH v5 10/17] PCI: dwc: Free the MSI domain after the host .deinit() callback
In reply to Koichiro DenHi Den-san,
Quoted 8 lines · Koichiro Den
dw_pcie_host_deinit()frees the iMSI-RX domain before calling the host .deinit() callback. For a core-owned parent interrupt,dw_pcie_free_msi()removes the chained handler right before the domain. A glue driver that owns the parent interrupt and demultiplexes the MSIs itself throughdw_handle_msi_irq()can only stop its handler in .deinit(), after the domain is gone, so a late interrupt can still dispatch into the freed domain.
Quoted 4 lines
Remove the domain after .deinit(), in
dw_pcie_host_deinit()and in thedw_pcie_host_init()unwind. Keep removing the chained handlers of core-owned interrupts before .deinit(), which powers the controller down. Clearpp->irq_domainwhen the domain is removed so the unwind can
FTR, this is now pp->msi_irq_domain since commit 8f0b8cc3a655f56b ("PCI: dwc: Rename pp->irq_domain to pp->msi_irq_domain") in pci/next.
Quoted 4 lines
tell whether it exists.
Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v5 10/17] PCI: dwc: Free the MSI domain after the host .deinit() callback
Quoted 11 lines · Geert Uytterhoeven
Hi Den-san,
Quoted 8 lines · Koichiro Den
dw_pcie_host_deinit()frees the iMSI-RX domain before calling the host .deinit() callback. For a core-owned parent interrupt,dw_pcie_free_msi()removes the chained handler right before the domain. A glue driver that owns the parent interrupt and demultiplexes the MSIs itself throughdw_handle_msi_irq()can only stop its handler in .deinit(), after the domain is gone, so a late interrupt can still dispatch into the freed domain.
Quoted 7 lines
Quoted 4 lines
Remove the domain after .deinit(), in
dw_pcie_host_deinit()and in thedw_pcie_host_init()unwind. Keep removing the chained handlers of core-owned interrupts before .deinit(), which powers the controller down. Clearpp->irq_domainwhen the domain is removed so the unwind canFTR, this is now
pp->msi_irq_domainsince commit8f0b8cc3a655f56b("PCI: dwc: Renamepp->irq_domaintopp->msi_irq_domain") in pci/next.
Yes, thanks! Shawn also pointed that out. I'll rebase once those changes reach linux-next, and update the commit message too.
Best regards,
Koichiro Den
Quoted 17 lines
Quoted 5 lines
tell whether it exists.
Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH v5 11/17 PCI: rcar-gen4: Take over the iMSI-RX interrupt
In reply to Koichiro DenOn R-Car Gen4 SoCs, intreq_pcim_sub ("msi") carries more than the integrated MSI receiver: the controller's reset requests and the Root Port's PME and bandwidth notifications are signalled on the same line, and the following patches need to handle them. With the DesignWare core owning the line through its chained handler, the driver would have to hook into that handler when iMSI-RX is used and request the line itself otherwise.
Instead, request the interrupt in the driver in all configurations and set pp->msi_irq[0] to -ENODEV so the core does not install its chained handler, as spear13xx, keembay and dra7xx do. The handler demultiplexes the MSIs through dw_handle_msi_irq() when the APP block reports msi_ctrl_int, in hard IRQ context like the chained handler did. With an external MSI controller or pci=nomsi the iMSI-RX is not set up, so keep msi_ctrl_int masked, and the handler has nothing to do there yet.
Request the interrupt in probe with IRQF_NO_AUTOEN and enable it at the end of .init(), before endpoint drivers can use MSIs from their probe. Disable it in .deinit(), before the controller is powered down, and around a Root Port reset, as the handler reads the controller registers. Suspend and resume go through the same two callbacks.
The DT routes downstream INTx to the same line, but the driver has never supported INTx (no INTx domain, INTx enables never set), so requesting the line exclusively takes nothing away.
---
Changes since v4:
- Add Marek's Reviewed-by tag. No code changes.
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 79 ++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-)
drivers/pci/controller/dwc/pcie-rcar-gen4.c +78 -1
@@ -115,6 +115,8 @@ struct rcar_gen4_pcie {struct platform_device *pdev;struct reset_control *perst;const struct rcar_gen4_pcie_drvdata *drvdata; +/* intreq_pcim_sub ("msi"): iMSI-RX and other controller notifications */ +int msi_irq;/* * The controller needs reinitializing; the APP interrupt sources * stay masked until a reset succeeds.
@@ -540,13 +542,14 @@ static int rcar_gen4_pcie_host_msi_addr(struct dw_pcie_rp *pp, u32 *msi_addr)static void rcar_gen4_pcie_app_irq_sync_locked (struct rcar_gen4_pcie *rcar) { +struct dw_pcie_rp *pp = &rcar->dw.pp;u32 val;lockdep_assert_held (&rcar->app_lock); val =readl (rcar->base +PCIEINTSTS0EN ); val &= ~MSI_CTRL_INT ; -if (!rcar->reinit_pending &&pci_msi_enabled ()) +if (!rcar->reinit_pending && pp->use_imsi_rx &&pci_msi_enabled ()) val |=MSI_CTRL_INT ;writel (val, rcar->base +PCIEINTSTS0EN ); }
@@ -612,6 +615,54 @@ static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)return ret; } +/* +* intreq_pcim_sub carries the iMSI-RX interrupt along with other controller +* notifications, so the driver owns it instead of the DesignWare core (see +* pp->msi_irq[0]) and demultiplexes the MSIs itself. +*/ +static irqreturn_t rcar_gen4_pcie_msi_irq_handler (int irq,void *data) +{ +struct rcar_gen4_pcie *rcar = data; +u32 status; + + status =readl (rcar->base +PCIEINTSTS0 ) & +readl (rcar->base +PCIEINTSTS0EN ); +if (!(status &MSI_CTRL_INT )) +return IRQ_NONE ; + +dw_handle_msi_irq (&rcar->dw.pp); + +return IRQ_HANDLED ; +} + +static int rcar_gen4_pcie_msi_irq_init (struct rcar_gen4_pcie *rcar) +{ +struct device *dev = rcar->dw.dev; +int irq, ret; + + irq =platform_get_irq_byname (rcar->pdev,"msi" ); +if (irq <0 ) +return irq; + +/* +* Child IRQ flow handlers require local IRQs to be disabled. Keep this +* handler in hard IRQ context on PREEMPT_RT. +* +* Keep the IRQ registered for the lifetime of the driver. .init() +* enables it once the controller is set up, and .deinit() disables it +* before the controller is powered down. +*/ + ret =devm_request_irq (dev, irq, rcar_gen4_pcie_msi_irq_handler, +IRQF_NO_THREAD |IRQF_NO_AUTOEN ,dev_name (dev), + rcar); +if (ret) +return dev_err_probe (dev, ret,"Failed to request \"msi\" IRQ\n" ); + + rcar->msi_irq = irq; + +return 0 ; +} +static int rcar_gen4_pcie_enable_device (struct pci_host_bridge *bridge,struct pci_dev *dev) {
@@ -709,6 +760,9 @@ static void rcar_gen4_pcie_quiesce_irqs(struct rcar_gen4_pcie *rcar) rcar->reinit_pending = true;rcar_gen4_pcie_app_irq_sync_locked (rcar); } + +/* The MSI status lives in DBI; keep the handler away during the reset. */ +disable_irq (rcar->msi_irq); } static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,
@@ -718,6 +772,8 @@ static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar, rcar->reinit_pending = !recovered;rcar_gen4_pcie_app_irq_sync_locked (rcar); } + +enable_irq (rcar->msi_irq); }/*
@@ -795,6 +851,13 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)if (ret) goto err; +/* +* Undo IRQF_NO_AUTOEN at probe, or the disable in .deinit() on resume. +* The iMSI-RX interrupts stay disabled until dw_pcie_setup_rc(), so the +* handler cannot run before the MSI domain exists. +*/ +rcar_gen4_pcie_resume_irqs (rcar, true); +return 0 ; err:
@@ -807,6 +870,9 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp)struct dw_pcie *dw =to_dw_pcie_from_pp (pp);struct rcar_gen4_pcie *rcar =to_rcar_gen4_pcie (dw); +/* Stop the handler before asserting reset and disabling the clocks. */ +rcar_gen4_pcie_quiesce_irqs (rcar); +rcar_gen4_pcie_host_perst_assert (pp, true); rcar->drvdata->deinit (rcar); }
@@ -819,15 +885,26 @@ static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {static int rcar_gen4_add_dw_pcie_rp (struct rcar_gen4_pcie *rcar) {struct dw_pcie_rp *pp = &rcar->dw.pp; +int ret;if (!IS_ENABLED (CONFIG_PCIE_RCAR_GEN4_HOST ))return -ENODEV ;raw_spin_lock_init (&rcar->app_lock); + ret =rcar_gen4_pcie_msi_irq_init (rcar); +if (ret) +return ret; + pp->num_vectors =MAX_MSI_IRQS ;/* Reserve an iATU window for the generic PME_Turn_Off implementation. */ pp->use_atu_msg = true; +/* +* intreq_pcim_sub ("msi") is shared with other controller +* notifications: the driver requests it itself and demultiplexes the +* iMSI-RX MSIs from its own handler. +*/ + pp->msi_irq[0 ] = -ENODEV ; pp->ops = &rcar_gen4_pcie_host_ops;return dw_pcie_host_init (pp);
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 11/17] PCI: rcar-gen4: Take over the iMSI-RX interrupt
Sashiko has reviewed this patch and found no issues.
Quoted 22 lines
On R-Car Gen4 SoCs,
intreq_pcim_sub("msi") carries more than the integrated MSI receiver: the controller's reset requests and the Root Port's PME and bandwidth notifications are signalled on the same line, and the following patches need to handle them. With the DesignWare core owning the line through its chained handler, the driver would have to hook into that handler when iMSI-RX is used and request the line itself otherwise.Instead, request the interrupt in the driver in all configurations and set pp->msi_irq[0] to
-ENODEVso the core does not install its chained handler, as spear13xx, keembay and dra7xx do. The handler demultiplexes the MSIs throughdw_handle_msi_irq()when the APP block reportsmsi_ctrl_int, in hard IRQ context like the chained handler did. With an external MSI controller or pci=nomsi the iMSI-RX is not set up, so keepmsi_ctrl_intmasked, and the handler has nothing to do there yet.Request the interrupt in probe with
IRQF_NO_AUTOENand enable it at the end of .init(), before endpoint drivers can use MSIs from their probe. Disable it in .deinit(), before the controller is powered down, and around a Root Port reset, as the handler reads the controller registers. Suspend and resume go through the same two callbacks.
Quoted 6 lines
The DT routes downstream INTx to the same line, but the driver has never supported INTx (no INTx domain, INTx enables never set), so requesting the line exclusively takes nothing away.
Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
PATCH v5 12/17 PCI: rcar-gen4: Recover the Root Port on link down
In reply to Koichiro DenOn R-Car, the controller's reset requests (smlh_req_rst_not, link_req_rst_not) arrive on intreq_pcim_sub, the interrupt the MSIs are demultiplexed from, so DBI is read as soon as the link goes down. On R-Car S4 that is a hazard: DBI accesses issued within a few hundred microseconds of an unexpected link down do not complete and hang the host. In testing, the first Root Port config read after powering off the link partner hung unless delayed by ~300 us.
Check the APP reset status in the interrupt handler before DBI is touched. When a reset request is latched, flag the controller as needing reinitialization, which masks the sources, ack the request and schedule recovery work. The work calls pci_host_handle_link_down() on the Root Port, which runs the AER-style recovery and resets the controller through reset_root_port(), holding a reference on the port. If the Root Port is absent when the work runs, reset the controller directly so that successful reinitialization can restore interrupt delivery. Hold the rescan lock across the absence check and reset. The APP reset-status check also reports a recovery already pending, whether started by another interrupt or left over from a failed reset, so the handler issues no further DBI accesses until a reset succeeds.
Arm detection after initial PCI enumeration, so recovery does not interrupt device discovery or resource assignment. Only unmasked status bits are handled and pending latches are cleared when the sources are re-enabled, so requests recorded during probe or the reset itself do not trigger another recovery. Teardown only disarms the detection: MSI delivery has to keep working while devices are removed.
Run recovery on a freezable workqueue to keep it outside device suspend/resume. Reject suspend while a reset request is pending, including one latched after the parent IRQs were suspended. dw_pcie_suspend_noirq() would otherwise access DBI, and the queued recovery can run once the system thaws. Disarm Root Port events during suspend and re-arm them in .post_init() after resume has set up the link, so taking the link down for suspend does not start recovery.
---
Changes since v4:
- Add Marek's Reviewed-by tag. No code changes.
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 127 ++++++++++++++++++++ 1 file changed, 127 insertions(+)
drivers/pci/controller/dwc/pcie-rcar-gen4.c +127 -0
@@ -25,6 +25,7 @@#include <linux/reset.h> #include "../../pci.h" +#include "../pci-host-common.h" #include "pcie-designware.h" /* Renesas-specific */
@@ -40,9 +41,18 @@#define MSICAP0_MMESCAP_MASK GENMASK(19, 17)#define MSICAP0_MSIE BIT(16) +/* PCIe Reset Status */ +#define PCIERSTSTS 0x0020 +#define SMLH_REQ_RST_NOT BIT(2) +#define LINK_REQ_RST_NOT BIT(1) +#define LINK_DOWN_RESET_MASK (SMLH_REQ_RST_NOT | LINK_REQ_RST_NOT) +/* PCIe Interrupt Status 0 */ #define PCIEINTSTS0 0x0084 +/* PCIe Reset Status Enable */ +#define PCIERSTSTSEN 0x0300 +/* PCIe Interrupt Status 0 Enable */ #define PCIEINTSTS0EN 0x0310#define MSI_CTRL_INT BIT(26)
@@ -53,6 +63,9 @@#define PCIEDMAINTSTSEN 0x0314#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0) +/* PCIe Reset Status Clear */ +#define PCIERSTSTSCLR 0x0330 +/* PCIe Interrupt Status 0 Clear */ #define PCIEINTSTS0CLR 0x0340
@@ -117,6 +130,9 @@ struct rcar_gen4_pcie {const struct rcar_gen4_pcie_drvdata *drvdata;/* intreq_pcim_sub ("msi"): iMSI-RX and other controller notifications */ int msi_irq; +struct work_struct link_down_work; +/* Allow Root Port event handling after enumeration and until teardown. */ +bool rp_events_armed;/* * The controller needs reinitializing; the APP interrupt sources * stay masked until a reset succeeds.
@@ -543,15 +559,35 @@ static int rcar_gen4_pcie_host_msi_addr(struct dw_pcie_rp *pp, u32 *msi_addr)static void rcar_gen4_pcie_app_irq_sync_locked (struct rcar_gen4_pcie *rcar) {struct dw_pcie_rp *pp = &rcar->dw.pp; +bool rp_events = rcar->rp_events_armed && !rcar->reinit_pending;u32 val;lockdep_assert_held (&rcar->app_lock); val =readl (rcar->base +PCIEINTSTS0EN ); val &= ~MSI_CTRL_INT ; +/* +* Note that rcar->rp_events_armed is not checked here because device +* removal may still need iMSI-RX interrupts after Root Port events have +* been disarmed. +*/ if (!rcar->reinit_pending && pp->use_imsi_rx &&pci_msi_enabled ()) val |=MSI_CTRL_INT ;writel (val, rcar->base +PCIEINTSTS0EN ); + + val =readl (rcar->base +PCIERSTSTSEN ); +if (rp_events) { +/* +* Clear latches recorded while the sources were masked, so +* stale requests do not fire as soon as they are re-enabled. +*/ +if (!(val &LINK_DOWN_RESET_MASK )) +writel (LINK_DOWN_RESET_MASK , rcar->base +PCIERSTSTSCLR ); + val |=LINK_DOWN_RESET_MASK ; + }else { + val &= ~LINK_DOWN_RESET_MASK ; + } +writel (val, rcar->base +PCIERSTSTSEN ); }static void rcar_gen4_pcie_app_irq_sync (struct rcar_gen4_pcie *rcar)
@@ -615,6 +651,50 @@ static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)return ret; } +/* +* Consume a latched reset request and start recovery. Returns true if DBI +* must be left alone: a request was just consumed or recovery is pending. +*/ +static bool rcar_gen4_pcie_handle_link_down (struct rcar_gen4_pcie *rcar) +{ +u32 status; + +guard (raw_spinlock_irqsave)(&rcar->app_lock); + + status =readl (rcar->base +PCIERSTSTS ) & +readl (rcar->base +PCIERSTSTSEN ) &LINK_DOWN_RESET_MASK ; +if (status) { +/* The sources are only enabled while armed with no reinit pending. */ + rcar->reinit_pending = true; +rcar_gen4_pcie_app_irq_sync_locked (rcar); +writel (status, rcar->base +PCIERSTSTSCLR ); +/* Keep recovery outside device suspend/resume. */ +queue_work (system_freezable_wq, &rcar->link_down_work); + } + +return rcar->reinit_pending; +} + +static void rcar_gen4_pcie_link_down_work (struct work_struct *work) +{ +struct rcar_gen4_pcie *rcar = + container_of(work, struct rcar_gen4_pcie, link_down_work); +struct pci_host_bridge *bridge = rcar->dw.pp.bridge; +struct pci_dev *port; + +/* Serialize the absence check and controller reset against rescans. */ +pci_lock_rescan_remove (); + port =pci_get_slot (bridge->bus,PCI_DEVFN (0 ,0 )); +if (!port) + bridge->reset_root_port (bridge,NULL ); +pci_unlock_rescan_remove (); + +if (port) { +pci_host_handle_link_down (port); +pci_dev_put (port); + } +} +/* * intreq_pcim_sub carries the iMSI-RX interrupt along with other controller * notifications, so the driver owns it instead of the DesignWare core (see
@@ -625,6 +705,10 @@ static irqreturn_t rcar_gen4_pcie_msi_irq_handler(int irq, void *data)struct rcar_gen4_pcie *rcar = data;u32 status; +/* Check reset requests before MSI handling, which accesses DBI. */ +if (rcar_gen4_pcie_handle_link_down (rcar)) +return IRQ_HANDLED ; + status =readl (rcar->base +PCIEINTSTS0 ) &readl (rcar->base +PCIEINTSTS0EN );if (!(status &MSI_CTRL_INT ))
@@ -663,6 +747,28 @@ static int rcar_gen4_pcie_msi_irq_init(struct rcar_gen4_pcie *rcar)return 0 ; } +/* Keep link-down recovery out of initial PCI enumeration. */ +static void rcar_gen4_pcie_rp_events_arm (struct dw_pcie_rp *pp) +{ +struct dw_pcie *dw =to_dw_pcie_from_pp (pp); +struct rcar_gen4_pcie *rcar =to_rcar_gen4_pcie (dw); + +guard (raw_spinlock_irqsave)(&rcar->app_lock); + rcar->rp_events_armed = true; +rcar_gen4_pcie_app_irq_sync_locked (rcar); +} + +static void rcar_gen4_pcie_rp_events_disarm (struct rcar_gen4_pcie *rcar) +{ + scoped_guard(raw_spinlock_irqsave, &rcar->app_lock) { + rcar->rp_events_armed = false; +rcar_gen4_pcie_app_irq_sync_locked (rcar); + } + +/* Handlers queue the work under app_lock, so none can follow this. */ +cancel_work_sync (&rcar->link_down_work); +} +static int rcar_gen4_pcie_enable_device (struct pci_host_bridge *bridge,struct pci_dev *dev) {
@@ -870,6 +976,9 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp)struct dw_pcie *dw =to_dw_pcie_from_pp (pp);struct rcar_gen4_pcie *rcar =to_rcar_gen4_pcie (dw); +/* Re-arm Root Port events in .post_init, after link setup. */ +rcar_gen4_pcie_rp_events_disarm (rcar); +/* Stop the handler before asserting reset and disabling the clocks. */ rcar_gen4_pcie_quiesce_irqs (rcar);
@@ -880,6 +989,7 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp)static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = { .init = rcar_gen4_pcie_host_init, .deinit = rcar_gen4_pcie_host_deinit, + .post_init = rcar_gen4_pcie_rp_events_arm, };static int rcar_gen4_add_dw_pcie_rp (struct rcar_gen4_pcie *rcar)
@@ -891,6 +1001,7 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)return -ENODEV ;raw_spin_lock_init (&rcar->app_lock); +INIT_WORK (&rcar->link_down_work, rcar_gen4_pcie_link_down_work); ret =rcar_gen4_pcie_msi_irq_init (rcar);if (ret)
@@ -912,6 +1023,8 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)static void rcar_gen4_remove_dw_pcie_rp (struct rcar_gen4_pcie *rcar) { +/* Cancel recovery work before removing the PCI bus */ +rcar_gen4_pcie_rp_events_disarm (rcar);dw_pcie_host_deinit (&rcar->dw.pp); }
@@ -1309,6 +1422,20 @@ static int rcar_gen4_pcie_suspend_noirq(struct device *dev)if (rcar->drvdata->mode !=DW_PCIE_RC_TYPE )return 0 ; +if (rcar->dw.suspended) +return 0 ; + +/* +* Recovery work queued after the workqueues were frozen cannot run +* until thaw. Abort suspend so it runs then, instead of touching DBI +* here. This also catches requests latched after the parent IRQs were +* suspended. +*/ +if (rcar_gen4_pcie_handle_link_down (rcar)) { +dev_warn (dev,"Controller needs reinitializing, aborting suspend\n" ); +return -EBUSY ; + } +return dw_pcie_suspend_noirq (&rcar->dw); }
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 12/17] PCI: rcar-gen4: Recover the Root Port on link down
Sashiko has reviewed this patch and found no issues.
Quoted 19 lines
On R-Car, the controller's reset requests (
smlh_req_rst_not,link_req_rst_not) arrive onintreq_pcim_sub, the interrupt the MSIs are demultiplexed from, so DBI is read as soon as the link goes down. On R-Car S4 that is a hazard: DBI accesses issued within a few hundred microseconds of an unexpected link down do not complete and hang the host. In testing, the first Root Port config read after powering off the link partner hung unless delayed by ~300 us.Check the APP reset status in the interrupt handler before DBI is touched. When a reset request is latched, flag the controller as needing reinitialization, which masks the sources, ack the request and schedule recovery work. The work calls
pci_host_handle_link_down()on the Root Port, which runs the AER-style recovery and resets the controller throughreset_root_port(), holding a reference on the port. If the Root Port is absent when the work runs, reset the controller [ ... ]
suspend and re-arm them in .post_init() after resume has set up the link, so taking the link down for suspend does not start recovery.
Quoted 2 lines
Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12
PATCH v5 13/17 PCI: rcar-gen4: Route Root Port AER to a virtual Root Port IRQ
In reply to Koichiro DenOn R-Car Gen4 SoCs, the APP block exposes the Root Port interrupt signals in PCIEINTSTS0 and routes them to platform IRQs: AER to intreq_pcim_lperr, and PME and bandwidth notifications to intreq_pcim_sub. On an R-Car S4 with GIC ITS, a link retrain set LnkSta.LBMS and latched PCIEINTSTS0[23] (cfg_bw_mgt_msi), but the Root Port's MSI never fired while endpoint MSIs worked fine. With iMSI-RX the Root Port MSI capabilities are already hidden, but its INTx maps to intreq_pcim_sub. Note that even before this driver took over the IRQ with rcar_gen4_pcie_msi_irq_handler, the port service was unable to request it, as the DWC core used that IRQ for its chained MSI handler. In either case, iMSI-RX or an external MSI controller, Root Port interrupts are not delivered to the port services.
Hide the Root Port MSI/MSI-X capabilities so the port services use INTx regardless of the MSI controller. Do this in the R-Car host setup, which also runs on reset and resume. Use a virtual IRQ to deliver notifications from separate platform IRQs to the standard handlers. Route AER from the optional "aer" interrupt. DTs without it remain supported, but Root Port AER stays unavailable.
The APP block may still receive MSI-form notifications because hiding the Root Port's MSI capabilities does not clear MSICAP0.MSIE. Handle these alongside the INTx-form level signals. Clear the MSI-form latch before the AER handler clears Root Error Status, so a late APP clear does not erase a new notification.
Check for reset requests before dispatch: on S4, DBI accesses from the port services can hang the host immediately after link down.
The next patch also feeds PME and bandwidth notifications from "msi" into this IRQ, so both parents may dispatch concurrently. handle_level_irq() calls irq_ack() before checking whether it can run the handlers. Keep the APP latches set while the IRQ is in progress or disabled, so the level parents can retry through the reset checks. This avoids losing notifications without a raw lock around port services that take regular spinlocks.
---
Changes since v4:
- Hide the Root Port MSI/MSI-X capabilities in host_setup() instead of adding a no_rp_msi flag to the DWC core. (Shawn)
https://lore.kernel.org/r/bi3rvyqpdu4iq6pfyrqvx7eplyim6gmg2ijal67mpateobmtlq@gec7zewfm76r/
I did not carry Marek's Reviewed-by tag over due to the change.
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 213 +++++++++++++++++++- 1 file changed, 210 insertions(+), 3 deletions(-)
drivers/pci/controller/dwc/pcie-rcar-gen4.c +210 -3
@@ -14,10 +14,12 @@#include <linux/io.h> #include <linux/iopoll.h> #include <linux/irqchip/arm-gic-v3.h> +#include <linux/irqdomain.h> #include <linux/module.h> #include <linux/of.h> #include <linux/of_address.h> #include <linux/of_irq.h> +#include <linux/of_pci.h> #include <linux/pci.h> #include <linux/phy/phy.h> #include <linux/platform_device.h>
@@ -59,6 +61,11 @@#define SMLH_LINK_UP BIT(7)#define RDLH_LINK_UP BIT(6) +/* Root Port notifications routed to intreq_pcim_lperr ("aer") */ +#define AER_RC_ERR_MSI BIT(29) +#define AER_RC_ERR_INT BIT(27) +#define RP_AER_MASK (AER_RC_ERR_MSI | AER_RC_ERR_INT) +/* PCIe DMA Interrupt Status Enable */ #define PCIEDMAINTSTSEN 0x0314#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)
@@ -68,6 +75,7 @@/* PCIe Interrupt Status 0 Clear */ #define PCIEINTSTS0CLR 0x0340 +#define RP_LATCHED_MASK AER_RC_ERR_MSI/* Port Logic Registers 89 */ #define PRTLGC89 0x0b70
@@ -133,6 +141,12 @@ struct rcar_gen4_pcie {struct work_struct link_down_work;/* Allow Root Port event handling after enumeration and until teardown. */ bool rp_events_armed; +/* Virtual Root Port INTx shared by the port services */ +struct irq_domain *rp_irq_domain; +unsigned int rp_irq; +bool rp_irq_enabled; +/* intreq_pcim_lperr ("aer"): Root Port AER notifications, optional */ +int aer_irq;/* * The controller needs reinitializing; the APP interrupt sources * stay masked until a reset succeeds.
@@ -565,7 +579,7 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar)lockdep_assert_held (&rcar->app_lock); val =readl (rcar->base +PCIEINTSTS0EN ); - val &= ~MSI_CTRL_INT ; + val &= ~(MSI_CTRL_INT |RP_AER_MASK );/* * Note that rcar->rp_events_armed is not checked here because device * removal may still need iMSI-RX interrupts after Root Port events have
@@ -573,6 +587,8 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar)*/ if (!rcar->reinit_pending && pp->use_imsi_rx &&pci_msi_enabled ()) val |=MSI_CTRL_INT ; +if (rp_events && rcar->rp_irq_enabled && rcar->aer_irq >0 ) + val |=RP_AER_MASK ;writel (val, rcar->base +PCIEINTSTS0EN ); val =readl (rcar->base +PCIERSTSTSEN );
@@ -675,6 +691,15 @@ static bool rcar_gen4_pcie_handle_link_down(struct rcar_gen4_pcie *rcar)return rcar->reinit_pending; } +static void rcar_gen4_pcie_rp_irq_dispatch (struct rcar_gen4_pcie *rcar) +{ +/* Recheck reset requests after the parent's status read. */ +if (rcar_gen4_pcie_handle_link_down (rcar)) +return ; + +generic_handle_domain_irq (rcar->rp_irq_domain,0 ); +} +static void rcar_gen4_pcie_link_down_work (struct work_struct *work) { struct rcar_gen4_pcie *rcar =
@@ -769,6 +794,162 @@ static void rcar_gen4_pcie_rp_events_disarm(struct rcar_gen4_pcie *rcar)cancel_work_sync (&rcar->link_down_work); } +static void rcar_gen4_pcie_rp_irq_mask (struct irq_data *d) +{ +struct rcar_gen4_pcie *rcar =irq_data_get_irq_chip_data (d); + +guard (raw_spinlock_irqsave)(&rcar->app_lock); + rcar->rp_irq_enabled = false; +rcar_gen4_pcie_app_irq_sync_locked (rcar); +} + +static void rcar_gen4_pcie_rp_irq_unmask (struct irq_data *d) +{ +struct rcar_gen4_pcie *rcar =irq_data_get_irq_chip_data (d); + +guard (raw_spinlock_irqsave)(&rcar->app_lock); + rcar->rp_irq_enabled = true; +rcar_gen4_pcie_app_irq_sync_locked (rcar); +} + +static void rcar_gen4_pcie_rp_irq_ack (struct irq_data *d) +{ +struct rcar_gen4_pcie *rcar =irq_data_get_irq_chip_data (d); + +/* +* handle_level_irq() may skip the handlers after calling irq_ack(). +* Keep notifications latched if the IRQ is disabled or in progress, +* so they raise a parent IRQ again after unmasking. The descriptor +* lock serializes this check with IRQD_IRQ_INPROGRESS. +*/ +if (irqd_irq_disabled (d) ||irqd_irq_inprogress (d)) +return ; + +/* Clear APP latches before the service clears Root Error Status. */ +writel (RP_LATCHED_MASK , rcar->base +PCIEINTSTS0CLR ); +} + +static const struct irq_chip rcar_gen4_pcie_rp_irq_chip = { + .name ="rcar-gen4-rp" , + .irq_mask = rcar_gen4_pcie_rp_irq_mask, + .irq_unmask = rcar_gen4_pcie_rp_irq_unmask, + .irq_ack = rcar_gen4_pcie_rp_irq_ack, +}; + +static int rcar_gen4_pcie_rp_irq_map (struct irq_domain *domain, +unsigned int irq,irq_hw_number_t hwirq) +{ +irq_set_chip_and_handler (irq, &rcar_gen4_pcie_rp_irq_chip, + handle_level_irq); +irq_set_chip_data (irq, domain->host_data); +irq_set_status_flags (irq,IRQ_LEVEL ); +irq_set_noprobe (irq); + +return 0 ; +} + +static const struct irq_domain_ops rcar_gen4_pcie_rp_irq_domain_ops = { + .map = rcar_gen4_pcie_rp_irq_map, +}; + +static irqreturn_t rcar_gen4_pcie_aer_irq_handler (int irq,void *data) +{ +struct rcar_gen4_pcie *rcar = data; +u32 status; + +/* The line also fires on link down; reset requests come first. */ +if (rcar_gen4_pcie_handle_link_down (rcar)) +return IRQ_HANDLED ; + + status =readl (rcar->base +PCIEINTSTS0 ) & +readl (rcar->base +PCIEINTSTS0EN ); +if (!(status &RP_AER_MASK )) +return IRQ_NONE ; + +rcar_gen4_pcie_rp_irq_dispatch (rcar); + +return IRQ_HANDLED ; +} + +static int rcar_gen4_pcie_map_irq (const struct pci_dev *dev,u8 slot,u8 pin) +{ +struct pci_host_bridge *bridge =pci_find_host_bridge (dev->bus); +struct rcar_gen4_pcie *rcar =dev_get_drvdata (bridge->dev.parent); + +/* Downstream devices use standard OF INTx mapping */ +if (!pci_is_root_bus (dev->bus)) +return of_irq_parse_and_map_pci (dev, slot, pin); + +/* Root Port uses the virtual IRQ (always INTA) */ +return rcar->rp_irq; +} + +static int rcar_gen4_pcie_rp_irq_init (struct rcar_gen4_pcie *rcar) +{ +struct device *dev = rcar->dw.dev; +struct fwnode_handle *fwnode; +int irq, ret; + + irq =platform_get_irq_byname_optional (rcar->pdev,"aer" ); +if (irq == -ENXIO ) + irq =0 ;/* old DT: Root Port AER stays unavailable */ +else if (irq <0 ) +return dev_err_probe (dev, irq,"Failed to get AER IRQ\n" ); + + fwnode =irq_domain_alloc_named_fwnode (dev_name (dev)); +if (!fwnode) +return -ENOMEM ; + + rcar->rp_irq_domain =irq_domain_create_linear (fwnode,1 , + &rcar_gen4_pcie_rp_irq_domain_ops, + rcar); +if (!rcar->rp_irq_domain) { + ret = -ENOMEM ; + goto err_free_fwnode; + } + + rcar->rp_irq =irq_create_mapping (rcar->rp_irq_domain,0 ); +if (!rcar->rp_irq) { + ret = -ENOMEM ; + goto err_remove_domain; + } + +if (irq) { +/* +* Dispatch the child IRQ in hard IRQ context, including on +* PREEMPT_RT. Enable it together with the "msi" parent IRQ. +*/ + ret =devm_request_irq (dev, irq, rcar_gen4_pcie_aer_irq_handler, +IRQF_NO_THREAD |IRQF_NO_AUTOEN , +"rcar-gen4-pcie-aer" , rcar); +if (ret) { +dev_err_probe (dev, ret,"Failed to request AER IRQ\n" ); + goto err_dispose_mapping; + } + rcar->aer_irq = irq; + } + +return 0 ; + +err_dispose_mapping: +irq_dispose_mapping (rcar->rp_irq); +err_remove_domain: +irq_domain_remove (rcar->rp_irq_domain); +err_free_fwnode: +irq_domain_free_fwnode (fwnode); + +return ret; +} + +static void rcar_gen4_pcie_rp_irq_deinit (struct rcar_gen4_pcie *rcar) +{ +struct fwnode_handle *fwnode = rcar->rp_irq_domain->fwnode; + +irq_dispose_mapping (rcar->rp_irq); +irq_domain_remove (rcar->rp_irq_domain); +irq_domain_free_fwnode (fwnode); +} +static int rcar_gen4_pcie_enable_device (struct pci_host_bridge *bridge,struct pci_dev *dev) {
@@ -854,6 +1035,13 @@ static int rcar_gen4_pcie_host_setup(struct dw_pcie_rp *pp)if (ret)return ret; +/* +* Root Port MSIs are not delivered. Hide the MSI/MSI-X capabilities +* so port services use the virtual INTx IRQ. +*/ +dw_pcie_remove_capability (dw,PCI_CAP_ID_MSI ); +dw_pcie_remove_capability (dw,PCI_CAP_ID_MSIX ); +msleep (PCIE_T_PVPERL_MS );/* pe_rst requires 100msec delay */ rcar_gen4_pcie_host_perst_assert (pp, false);
@@ -869,6 +1057,8 @@ static void rcar_gen4_pcie_quiesce_irqs(struct rcar_gen4_pcie *rcar)/* The MSI status lives in DBI; keep the handler away during the reset. */ disable_irq (rcar->msi_irq); +if (rcar->aer_irq >0 ) +disable_irq (rcar->aer_irq); } static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,
@@ -879,6 +1069,8 @@ static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,rcar_gen4_pcie_app_irq_sync_locked (rcar); } +if (rcar->aer_irq >0 ) +enable_irq (rcar->aer_irq);enable_irq (rcar->msi_irq); }
@@ -945,6 +1137,8 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)if (pp->bridge) { pp->bridge->enable_device = rcar_gen4_pcie_enable_device; pp->bridge->reset_root_port = rcar_gen4_pcie_reset_root_port; +/* Route Root Port INTx to the virtual IRQ. */ + pp->bridge->map_irq = rcar_gen4_pcie_map_irq; }rcar_gen4_pcie_host_perst_assert (pp, true);
@@ -1003,10 +1197,14 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)raw_spin_lock_init (&rcar->app_lock);INIT_WORK (&rcar->link_down_work, rcar_gen4_pcie_link_down_work); - ret =rcar_gen4_pcie_msi_irq_init (rcar); + ret =rcar_gen4_pcie_rp_irq_init (rcar);if (ret)return ret; + ret =rcar_gen4_pcie_msi_irq_init (rcar); +if (ret) + goto err_rp_irq_deinit; + pp->num_vectors =MAX_MSI_IRQS ;/* Reserve an iATU window for the generic PME_Turn_Off implementation. */ pp->use_atu_msg = true;
@@ -1018,7 +1216,15 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar) pp->msi_irq[0 ] = -ENODEV ; pp->ops = &rcar_gen4_pcie_host_ops; -return dw_pcie_host_init (pp); + ret =dw_pcie_host_init (pp); +if (ret) + goto err_rp_irq_deinit; + +return 0 ; + +err_rp_irq_deinit: +rcar_gen4_pcie_rp_irq_deinit (rcar); +return ret; }static void rcar_gen4_remove_dw_pcie_rp (struct rcar_gen4_pcie *rcar)
@@ -1026,6 +1232,7 @@ static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)/* Cancel recovery work before removing the PCI bus */ rcar_gen4_pcie_rp_events_disarm (rcar);dw_pcie_host_deinit (&rcar->dw.pp); +rcar_gen4_pcie_rp_irq_deinit (rcar); }/* Endpoint mode */
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 13/17] PCI: rcar-gen4: Route Root Port AER to a virtual Root Port IRQ
Sashiko has reviewed this patch and found no issues.
Quoted 13 lines
On R-Car Gen4 SoCs, the APP block exposes the Root Port interrupt signals in PCIEINTSTS0 and routes them to platform IRQs: AER to
intreq_pcim_lperr, and PME and bandwidth notifications tointreq_pcim_sub. On an R-Car S4 with GIC ITS, a link retrain set LnkSta.LBMS and latched PCIEINTSTS0[23] (cfg_bw_mgt_msi), but the Root Port's MSI never fired while endpoint MSIs worked fine. With iMSI-RX the Root Port MSI capabilities are already hidden, but its INTx maps tointreq_pcim_sub. Note that even before this driver took over the IRQ withrcar_gen4_pcie_msi_irq_handler, the port service was unable to request it, as the DWC core used that IRQ for its chained MSI handler. In either case, iMSI-RX or an external MSI controller, Root Port interrupts are not delivered to the port services.
Quoted 8 lines
Hide the Root Port MSI/MSI-X capabilities so the port services use INTx regardless of the MSI controller. Do this in the R-Car host setup, [ ... ]
disabled, so the level parents can retry through the reset checks. This avoids losing notifications without a raw lock around port services that take regular spinlocks.Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13
Re: [PATCH v5 13/17] PCI: rcar-gen4: Route Root Port AER to a virtual Root Port IRQ
In reply to Koichiro DenQuoted 37 lines · Koichiro Den
On R-Car Gen4 SoCs, the APP block exposes the Root Port interrupt signals in PCIEINTSTS0 and routes them to platform IRQs: AER to
intreq_pcim_lperr, and PME and bandwidth notifications tointreq_pcim_sub. On an R-Car S4 with GIC ITS, a link retrain set LnkSta.LBMS and latched PCIEINTSTS0[23] (cfg_bw_mgt_msi), but the Root Port's MSI never fired while endpoint MSIs worked fine. With iMSI-RX the Root Port MSI capabilities are already hidden, but its INTx maps tointreq_pcim_sub. Note that even before this driver took over the IRQ withrcar_gen4_pcie_msi_irq_handler, the port service was unable to request it, as the DWC core used that IRQ for its chained MSI handler. In either case, iMSI-RX or an external MSI controller, Root Port interrupts are not delivered to the port services.Hide the Root Port MSI/MSI-X capabilities so the port services use INTx regardless of the MSI controller. Do this in the R-Car host setup, which also runs on reset and resume. Use a virtual IRQ to deliver notifications from separate platform IRQs to the standard handlers. Route AER from the optional "aer" interrupt. DTs without it remain supported, but Root Port AER stays unavailable.
The APP block may still receive MSI-form notifications because hiding the Root Port's MSI capabilities does not clear MSICAP0.MSIE. Handle these alongside the INTx-form level signals. Clear the MSI-form latch before the AER handler clears Root Error Status, so a late APP clear does not erase a new notification.
Check for reset requests before dispatch: on S4, DBI accesses from the port services can hang the host immediately after link down.
The next patch also feeds PME and bandwidth notifications from "msi" into this IRQ, so both parents may dispatch concurrently.
handle_level_irq()callsirq_ack()before checking whether it can run the handlers. Keep the APP latches set while the IRQ is in progress or disabled, so the level parents can retry through the reset checks. This avoids losing notifications without a raw lock around port services that take regular spinlocks.
Quoted 7 lines
Signed-off-by Koichiro Den <[email protected]>---
Changes since v4:
- Hide the Root Port MSI/MSI-X capabilities inhost_setup()instead of adding ano_rp_msiflag to the DWC core. (Shawn)
https://lore.kernel.org/r/bi3rvyqpdu4iq6pfyrqvx7eplyim6gmg2ijal67mpateobmtlq@gec7zewfm76r/
I did not carry Marek's Reviewed-by tag over due to the change.
Bjorn/Manivannan, can you please have a look at this patch and the MSI handling, to be sure this is OK ?
(no further comments from me below this point)
Thank you !
Quoted 332 lines
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 213 +++++++++++++++++++- 1 file changed, 210 insertions(+), 3 deletions(-)drivers/pci/controller/dwc/pcie-rcar-gen4.c +210
-3@@ -14,10 +14,12 @@#include <linux/io.h> #include <linux/iopoll.h> #include <linux/irqchip/arm-gic-v3.h> +#include <linux/irqdomain.h> #include <linux/module.h> #include <linux/of.h> #include <linux/of_address.h> #include <linux/of_irq.h> +#include <linux/of_pci.h> #include <linux/pci.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> @@ -59,6 +61,11 @@#define SMLH_LINK_UP BIT(7)#define RDLH_LINK_UP BIT(6) +/* Root Port notifications routed to intreq_pcim_lperr ("aer") */ +#define AER_RC_ERR_MSI BIT(29) +#define AER_RC_ERR_INT BIT(27) +#define RP_AER_MASK (AER_RC_ERR_MSI | AER_RC_ERR_INT) +/* PCIe DMA Interrupt Status Enable */ #define PCIEDMAINTSTSEN 0x0314#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)@@ -68,6 +75,7 @@/* PCIe Interrupt Status 0 Clear */ #define PCIEINTSTS0CLR 0x0340 +#define RP_LATCHED_MASK AER_RC_ERR_MSI/* Port Logic Registers 89 */ #define PRTLGC89 0x0b70@@ -133,6 +141,12 @@ struct rcar_gen4_pcie {struct work_struct link_down_work;/* Allow Root Port event handling after enumeration and until teardown. */ bool rp_events_armed; +/* Virtual Root Port INTx shared by the port services */ +struct irq_domain *rp_irq_domain; +unsigned int rp_irq; +bool rp_irq_enabled; +/* intreq_pcim_lperr ("aer"): Root Port AER notifications, optional */ +int aer_irq;/* * The controller needs reinitializing; the APP interrupt sources * stay masked until a reset succeeds. @@ -565,7 +579,7 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar)lockdep_assert_held (&rcar->app_lock); val =readl (rcar->base +PCIEINTSTS0EN ); - val &= ~MSI_CTRL_INT ; + val &= ~(MSI_CTRL_INT |RP_AER_MASK );/* * Note that rcar->rp_events_armed is not checked here because device * removal may still need iMSI-RX interrupts after Root Port events have @@ -573,6 +587,8 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar)*/ if (!rcar->reinit_pending && pp->use_imsi_rx &&pci_msi_enabled ()) val |=MSI_CTRL_INT ; +if (rp_events && rcar->rp_irq_enabled && rcar->aer_irq >0 ) + val |=RP_AER_MASK ;writel (val, rcar->base +PCIEINTSTS0EN ); val =readl (rcar->base +PCIERSTSTSEN );@@ -675,6 +691,15 @@ static bool rcar_gen4_pcie_handle_link_down(struct rcar_gen4_pcie *rcar)return rcar->reinit_pending; } +static void rcar_gen4_pcie_rp_irq_dispatch (struct rcar_gen4_pcie *rcar) +{ +/* Recheck reset requests after the parent's status read. */ +if (rcar_gen4_pcie_handle_link_down (rcar)) +return ; + +generic_handle_domain_irq (rcar->rp_irq_domain,0 ); +} +static void rcar_gen4_pcie_link_down_work (struct work_struct *work) { struct rcar_gen4_pcie *rcar =@@ -769,6 +794,162 @@ static void rcar_gen4_pcie_rp_events_disarm(struct rcar_gen4_pcie *rcar)cancel_work_sync (&rcar->link_down_work); } +static void rcar_gen4_pcie_rp_irq_mask (struct irq_data *d) +{ +struct rcar_gen4_pcie *rcar =irq_data_get_irq_chip_data (d); + +guard (raw_spinlock_irqsave)(&rcar->app_lock); + rcar->rp_irq_enabled = false; +rcar_gen4_pcie_app_irq_sync_locked (rcar); +} + +static void rcar_gen4_pcie_rp_irq_unmask (struct irq_data *d) +{ +struct rcar_gen4_pcie *rcar =irq_data_get_irq_chip_data (d); + +guard (raw_spinlock_irqsave)(&rcar->app_lock); + rcar->rp_irq_enabled = true; +rcar_gen4_pcie_app_irq_sync_locked (rcar); +} + +static void rcar_gen4_pcie_rp_irq_ack (struct irq_data *d) +{ +struct rcar_gen4_pcie *rcar =irq_data_get_irq_chip_data (d); + +/* +* handle_level_irq() may skip the handlers after calling irq_ack(). +* Keep notifications latched if the IRQ is disabled or in progress, +* so they raise a parent IRQ again after unmasking. The descriptor +* lock serializes this check with IRQD_IRQ_INPROGRESS. +*/ +if (irqd_irq_disabled (d) ||irqd_irq_inprogress (d)) +return ; + +/* Clear APP latches before the service clears Root Error Status. */ +writel (RP_LATCHED_MASK , rcar->base +PCIEINTSTS0CLR ); +} + +static const struct irq_chip rcar_gen4_pcie_rp_irq_chip = { + .name ="rcar-gen4-rp" , + .irq_mask = rcar_gen4_pcie_rp_irq_mask, + .irq_unmask = rcar_gen4_pcie_rp_irq_unmask, + .irq_ack = rcar_gen4_pcie_rp_irq_ack, +}; + +static int rcar_gen4_pcie_rp_irq_map (struct irq_domain *domain, +unsigned int irq,irq_hw_number_t hwirq) +{ +irq_set_chip_and_handler (irq, &rcar_gen4_pcie_rp_irq_chip, + handle_level_irq); +irq_set_chip_data (irq, domain->host_data); +irq_set_status_flags (irq,IRQ_LEVEL ); +irq_set_noprobe (irq); + +return 0 ; +} + +static const struct irq_domain_ops rcar_gen4_pcie_rp_irq_domain_ops = { + .map = rcar_gen4_pcie_rp_irq_map, +}; + +static irqreturn_t rcar_gen4_pcie_aer_irq_handler (int irq,void *data) +{ +struct rcar_gen4_pcie *rcar = data; +u32 status; + +/* The line also fires on link down; reset requests come first. */ +if (rcar_gen4_pcie_handle_link_down (rcar)) +return IRQ_HANDLED ; + + status =readl (rcar->base +PCIEINTSTS0 ) & +readl (rcar->base +PCIEINTSTS0EN ); +if (!(status &RP_AER_MASK )) +return IRQ_NONE ; + +rcar_gen4_pcie_rp_irq_dispatch (rcar); + +return IRQ_HANDLED ; +} + +static int rcar_gen4_pcie_map_irq (const struct pci_dev *dev,u8 slot,u8 pin) +{ +struct pci_host_bridge *bridge =pci_find_host_bridge (dev->bus); +struct rcar_gen4_pcie *rcar =dev_get_drvdata (bridge->dev.parent); + +/* Downstream devices use standard OF INTx mapping */ +if (!pci_is_root_bus (dev->bus)) +return of_irq_parse_and_map_pci (dev, slot, pin); + +/* Root Port uses the virtual IRQ (always INTA) */ +return rcar->rp_irq; +} + +static int rcar_gen4_pcie_rp_irq_init (struct rcar_gen4_pcie *rcar) +{ +struct device *dev = rcar->dw.dev; +struct fwnode_handle *fwnode; +int irq, ret; + + irq =platform_get_irq_byname_optional (rcar->pdev,"aer" ); +if (irq == -ENXIO ) + irq =0 ;/* old DT: Root Port AER stays unavailable */ +else if (irq <0 ) +return dev_err_probe (dev, irq,"Failed to get AER IRQ\n" ); + + fwnode =irq_domain_alloc_named_fwnode (dev_name (dev)); +if (!fwnode) +return -ENOMEM ; + + rcar->rp_irq_domain =irq_domain_create_linear (fwnode,1 , + &rcar_gen4_pcie_rp_irq_domain_ops, + rcar); +if (!rcar->rp_irq_domain) { + ret = -ENOMEM ; + goto err_free_fwnode; + } + + rcar->rp_irq =irq_create_mapping (rcar->rp_irq_domain,0 ); +if (!rcar->rp_irq) { + ret = -ENOMEM ; + goto err_remove_domain; + } + +if (irq) { +/* +* Dispatch the child IRQ in hard IRQ context, including on +* PREEMPT_RT. Enable it together with the "msi" parent IRQ. +*/ + ret =devm_request_irq (dev, irq, rcar_gen4_pcie_aer_irq_handler, +IRQF_NO_THREAD |IRQF_NO_AUTOEN , +"rcar-gen4-pcie-aer" , rcar); +if (ret) { +dev_err_probe (dev, ret,"Failed to request AER IRQ\n" ); + goto err_dispose_mapping; + } + rcar->aer_irq = irq; + } + +return 0 ; + +err_dispose_mapping: +irq_dispose_mapping (rcar->rp_irq); +err_remove_domain: +irq_domain_remove (rcar->rp_irq_domain); +err_free_fwnode: +irq_domain_free_fwnode (fwnode); + +return ret; +} + +static void rcar_gen4_pcie_rp_irq_deinit (struct rcar_gen4_pcie *rcar) +{ +struct fwnode_handle *fwnode = rcar->rp_irq_domain->fwnode; + +irq_dispose_mapping (rcar->rp_irq); +irq_domain_remove (rcar->rp_irq_domain); +irq_domain_free_fwnode (fwnode); +} +static int rcar_gen4_pcie_enable_device (struct pci_host_bridge *bridge,struct pci_dev *dev) {@@ -854,6 +1035,13 @@ static int rcar_gen4_pcie_host_setup(struct dw_pcie_rp *pp)if (ret)return ret; +/* +* Root Port MSIs are not delivered. Hide the MSI/MSI-X capabilities +* so port services use the virtual INTx IRQ. +*/ +dw_pcie_remove_capability (dw,PCI_CAP_ID_MSI ); +dw_pcie_remove_capability (dw,PCI_CAP_ID_MSIX ); +msleep (PCIE_T_PVPERL_MS );/* pe_rst requires 100msec delay */ rcar_gen4_pcie_host_perst_assert (pp, false);@@ -869,6 +1057,8 @@ static void rcar_gen4_pcie_quiesce_irqs(struct rcar_gen4_pcie *rcar)/* The MSI status lives in DBI; keep the handler away during the reset. */ disable_irq (rcar->msi_irq); +if (rcar->aer_irq >0 ) +disable_irq (rcar->aer_irq); } static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,@@ -879,6 +1069,8 @@ static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,rcar_gen4_pcie_app_irq_sync_locked (rcar); } +if (rcar->aer_irq >0 ) +enable_irq (rcar->aer_irq);enable_irq (rcar->msi_irq); }@@ -945,6 +1137,8 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)if (pp->bridge) { pp->bridge->enable_device = rcar_gen4_pcie_enable_device; pp->bridge->reset_root_port = rcar_gen4_pcie_reset_root_port; +/* Route Root Port INTx to the virtual IRQ. */ + pp->bridge->map_irq = rcar_gen4_pcie_map_irq; }rcar_gen4_pcie_host_perst_assert (pp, true);@@ -1003,10 +1197,14 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)raw_spin_lock_init (&rcar->app_lock);INIT_WORK (&rcar->link_down_work, rcar_gen4_pcie_link_down_work); - ret =rcar_gen4_pcie_msi_irq_init (rcar); + ret =rcar_gen4_pcie_rp_irq_init (rcar);if (ret)return ret; + ret =rcar_gen4_pcie_msi_irq_init (rcar); +if (ret) + goto err_rp_irq_deinit; + pp->num_vectors =MAX_MSI_IRQS ;/* Reserve an iATU window for the generic PME_Turn_Off implementation. */ pp->use_atu_msg = true;@@ -1018,7 +1216,15 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar) pp->msi_irq[0 ] = -ENODEV ; pp->ops = &rcar_gen4_pcie_host_ops; -return dw_pcie_host_init (pp); + ret =dw_pcie_host_init (pp); +if (ret) + goto err_rp_irq_deinit; + +return 0 ; + +err_rp_irq_deinit: +rcar_gen4_pcie_rp_irq_deinit (rcar); +return ret; }static void rcar_gen4_remove_dw_pcie_rp (struct rcar_gen4_pcie *rcar)@@ -1026,6 +1232,7 @@ static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)/* Cancel recovery work before removing the PCI bus */ rcar_gen4_pcie_rp_events_disarm (rcar);dw_pcie_host_deinit (&rcar->dw.pp); +rcar_gen4_pcie_rp_irq_deinit (rcar); }/* Endpoint mode */
Signature · 2 lines
Best regards, Marek Vasut
Re: [PATCH v5 13/17] PCI: rcar-gen4: Route Root Port AER to a virtual Root Port IRQ
Quoted 46 lines · Marek Vasut
Quoted 37 lines · Koichiro Den
On R-Car Gen4 SoCs, the APP block exposes the Root Port interrupt signals in PCIEINTSTS0 and routes them to platform IRQs: AER to
intreq_pcim_lperr, and PME and bandwidth notifications tointreq_pcim_sub. On an R-Car S4 with GIC ITS, a link retrain set LnkSta.LBMS and latched PCIEINTSTS0[23] (cfg_bw_mgt_msi), but the Root Port's MSI never fired while endpoint MSIs worked fine. With iMSI-RX the Root Port MSI capabilities are already hidden, but its INTx maps tointreq_pcim_sub. Note that even before this driver took over the IRQ withrcar_gen4_pcie_msi_irq_handler, the port service was unable to request it, as the DWC core used that IRQ for its chained MSI handler. In either case, iMSI-RX or an external MSI controller, Root Port interrupts are not delivered to the port services.Hide the Root Port MSI/MSI-X capabilities so the port services use INTx regardless of the MSI controller. Do this in the R-Car host setup, which also runs on reset and resume. Use a virtual IRQ to deliver notifications from separate platform IRQs to the standard handlers. Route AER from the optional "aer" interrupt. DTs without it remain supported, but Root Port AER stays unavailable.
The APP block may still receive MSI-form notifications because hiding the Root Port's MSI capabilities does not clear MSICAP0.MSIE. Handle these alongside the INTx-form level signals. Clear the MSI-form latch before the AER handler clears Root Error Status, so a late APP clear does not erase a new notification.
Check for reset requests before dispatch: on S4, DBI accesses from the port services can hang the host immediately after link down.
The next patch also feeds PME and bandwidth notifications from "msi" into this IRQ, so both parents may dispatch concurrently.
handle_level_irq()callsirq_ack()before checking whether it can run the handlers. Keep the APP latches set while the IRQ is in progress or disabled, so the level parents can retry through the reset checks. This avoids losing notifications without a raw lock around port services that take regular spinlocks.Quoted 7 lines
Signed-off-by Koichiro Den <[email protected]>---
Changes since v4:
- Hide the Root Port MSI/MSI-X capabilities inhost_setup()instead of adding ano_rp_msiflag to the DWC core. (Shawn)
https://lore.kernel.org/r/bi3rvyqpdu4iq6pfyrqvx7eplyim6gmg2ijal67mpateobmtlq@gec7zewfm76r/
I did not carry Marek's Reviewed-by tag over due to the change.
Quoted 6 lines
Bjorn/Manivannan, can you please have a look at this patch and the MSI handling, to be sure this is OK ?
(no further comments from me below this point)
Thank you !
Marek, thanks for the review.
Bjorn/Mani, just a gentle ping. Could one of you take a look? To summarize:
- v4 had 'no_rp_msi' flag to the DWC core. Shawn suggested keeping that in the glue driver instead, which I found reasonable:
https://lore.kernel.org/r/[email protected]/
- v5 hides them in rcar_gen4_pcie_host_setup(). (this patch)
If you'd rather I send v6 first, rebased on the pp->msi_irq_domain rename Shawn and Geert pointed out, please let me know. Otherwise I'll wait for your comments before respinning.
Best regards,
Koichiro Den
Quoted 319 lines
Quoted 314 lines
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 213 +++++++++++++++++++- 1 file changed, 210 insertions(+), 3 deletions(-)drivers/pci/controller/dwc/pcie-rcar-gen4.c +210
-3@@ -14,10 +14,12 @@#include <linux/io.h> #include <linux/iopoll.h> #include <linux/irqchip/arm-gic-v3.h> +#include <linux/irqdomain.h> #include <linux/module.h> #include <linux/of.h> #include <linux/of_address.h> #include <linux/of_irq.h> +#include <linux/of_pci.h> #include <linux/pci.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> @@ -59,6 +61,11 @@#define SMLH_LINK_UP BIT(7)#define RDLH_LINK_UP BIT(6) +/* Root Port notifications routed to intreq_pcim_lperr ("aer") */ +#define AER_RC_ERR_MSI BIT(29) +#define AER_RC_ERR_INT BIT(27) +#define RP_AER_MASK (AER_RC_ERR_MSI | AER_RC_ERR_INT) +/* PCIe DMA Interrupt Status Enable */ #define PCIEDMAINTSTSEN 0x0314#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)@@ -68,6 +75,7 @@/* PCIe Interrupt Status 0 Clear */ #define PCIEINTSTS0CLR 0x0340 +#define RP_LATCHED_MASK AER_RC_ERR_MSI/* Port Logic Registers 89 */ #define PRTLGC89 0x0b70@@ -133,6 +141,12 @@ struct rcar_gen4_pcie {struct work_struct link_down_work;/* Allow Root Port event handling after enumeration and until teardown. */ bool rp_events_armed; +/* Virtual Root Port INTx shared by the port services */ +struct irq_domain *rp_irq_domain; +unsigned int rp_irq; +bool rp_irq_enabled; +/* intreq_pcim_lperr ("aer"): Root Port AER notifications, optional */ +int aer_irq;/* * The controller needs reinitializing; the APP interrupt sources * stay masked until a reset succeeds. @@ -565,7 +579,7 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar)lockdep_assert_held (&rcar->app_lock); val =readl (rcar->base +PCIEINTSTS0EN ); - val &= ~MSI_CTRL_INT ; + val &= ~(MSI_CTRL_INT |RP_AER_MASK );/* * Note that rcar->rp_events_armed is not checked here because device * removal may still need iMSI-RX interrupts after Root Port events have @@ -573,6 +587,8 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar)*/ if (!rcar->reinit_pending && pp->use_imsi_rx &&pci_msi_enabled ()) val |=MSI_CTRL_INT ; +if (rp_events && rcar->rp_irq_enabled && rcar->aer_irq >0 ) + val |=RP_AER_MASK ;writel (val, rcar->base +PCIEINTSTS0EN ); val =readl (rcar->base +PCIERSTSTSEN );@@ -675,6 +691,15 @@ static bool rcar_gen4_pcie_handle_link_down(struct rcar_gen4_pcie *rcar)return rcar->reinit_pending; } +static void rcar_gen4_pcie_rp_irq_dispatch (struct rcar_gen4_pcie *rcar) +{ +/* Recheck reset requests after the parent's status read. */ +if (rcar_gen4_pcie_handle_link_down (rcar)) +return ; + +generic_handle_domain_irq (rcar->rp_irq_domain,0 ); +} +static void rcar_gen4_pcie_link_down_work (struct work_struct *work) { struct rcar_gen4_pcie *rcar =@@ -769,6 +794,162 @@ static void rcar_gen4_pcie_rp_events_disarm(struct rcar_gen4_pcie *rcar)cancel_work_sync (&rcar->link_down_work); } +static void rcar_gen4_pcie_rp_irq_mask (struct irq_data *d) +{ +struct rcar_gen4_pcie *rcar =irq_data_get_irq_chip_data (d); + +guard (raw_spinlock_irqsave)(&rcar->app_lock); + rcar->rp_irq_enabled = false; +rcar_gen4_pcie_app_irq_sync_locked (rcar); +} + +static void rcar_gen4_pcie_rp_irq_unmask (struct irq_data *d) +{ +struct rcar_gen4_pcie *rcar =irq_data_get_irq_chip_data (d); + +guard (raw_spinlock_irqsave)(&rcar->app_lock); + rcar->rp_irq_enabled = true; +rcar_gen4_pcie_app_irq_sync_locked (rcar); +} + +static void rcar_gen4_pcie_rp_irq_ack (struct irq_data *d) +{ +struct rcar_gen4_pcie *rcar =irq_data_get_irq_chip_data (d); + +/* +* handle_level_irq() may skip the handlers after calling irq_ack(). +* Keep notifications latched if the IRQ is disabled or in progress, +* so they raise a parent IRQ again after unmasking. The descriptor +* lock serializes this check with IRQD_IRQ_INPROGRESS. +*/ +if (irqd_irq_disabled (d) ||irqd_irq_inprogress (d)) +return ; + +/* Clear APP latches before the service clears Root Error Status. */ +writel (RP_LATCHED_MASK , rcar->base +PCIEINTSTS0CLR ); +} + +static const struct irq_chip rcar_gen4_pcie_rp_irq_chip = { + .name ="rcar-gen4-rp" , + .irq_mask = rcar_gen4_pcie_rp_irq_mask, + .irq_unmask = rcar_gen4_pcie_rp_irq_unmask, + .irq_ack = rcar_gen4_pcie_rp_irq_ack, +}; + +static int rcar_gen4_pcie_rp_irq_map (struct irq_domain *domain, +unsigned int irq,irq_hw_number_t hwirq) +{ +irq_set_chip_and_handler (irq, &rcar_gen4_pcie_rp_irq_chip, + handle_level_irq); +irq_set_chip_data (irq, domain->host_data); +irq_set_status_flags (irq,IRQ_LEVEL ); +irq_set_noprobe (irq); + +return 0 ; +} + +static const struct irq_domain_ops rcar_gen4_pcie_rp_irq_domain_ops = { + .map = rcar_gen4_pcie_rp_irq_map, +}; + +static irqreturn_t rcar_gen4_pcie_aer_irq_handler (int irq,void *data) +{ +struct rcar_gen4_pcie *rcar = data; +u32 status; + +/* The line also fires on link down; reset requests come first. */ +if (rcar_gen4_pcie_handle_link_down (rcar)) +return IRQ_HANDLED ; + + status =readl (rcar->base +PCIEINTSTS0 ) & +readl (rcar->base +PCIEINTSTS0EN ); +if (!(status &RP_AER_MASK )) +return IRQ_NONE ; + +rcar_gen4_pcie_rp_irq_dispatch (rcar); + +return IRQ_HANDLED ; +} + +static int rcar_gen4_pcie_map_irq (const struct pci_dev *dev,u8 slot,u8 pin) +{ +struct pci_host_bridge *bridge =pci_find_host_bridge (dev->bus); +struct rcar_gen4_pcie *rcar =dev_get_drvdata (bridge->dev.parent); + +/* Downstream devices use standard OF INTx mapping */ +if (!pci_is_root_bus (dev->bus)) +return of_irq_parse_and_map_pci (dev, slot, pin); + +/* Root Port uses the virtual IRQ (always INTA) */ +return rcar->rp_irq; +} + +static int rcar_gen4_pcie_rp_irq_init (struct rcar_gen4_pcie *rcar) +{ +struct device *dev = rcar->dw.dev; +struct fwnode_handle *fwnode; +int irq, ret; + + irq =platform_get_irq_byname_optional (rcar->pdev,"aer" ); +if (irq == -ENXIO ) + irq =0 ;/* old DT: Root Port AER stays unavailable */ +else if (irq <0 ) +return dev_err_probe (dev, irq,"Failed to get AER IRQ\n" ); + + fwnode =irq_domain_alloc_named_fwnode (dev_name (dev)); +if (!fwnode) +return -ENOMEM ; + + rcar->rp_irq_domain =irq_domain_create_linear (fwnode,1 , + &rcar_gen4_pcie_rp_irq_domain_ops, + rcar); +if (!rcar->rp_irq_domain) { + ret = -ENOMEM ; + goto err_free_fwnode; + } + + rcar->rp_irq =irq_create_mapping (rcar->rp_irq_domain,0 ); +if (!rcar->rp_irq) { + ret = -ENOMEM ; + goto err_remove_domain; + } + +if (irq) { +/* +* Dispatch the child IRQ in hard IRQ context, including on +* PREEMPT_RT. Enable it together with the "msi" parent IRQ. +*/ + ret =devm_request_irq (dev, irq, rcar_gen4_pcie_aer_irq_handler, +IRQF_NO_THREAD |IRQF_NO_AUTOEN , +"rcar-gen4-pcie-aer" , rcar); +if (ret) { +dev_err_probe (dev, ret,"Failed to request AER IRQ\n" ); + goto err_dispose_mapping; + } + rcar->aer_irq = irq; + } + +return 0 ; + +err_dispose_mapping: +irq_dispose_mapping (rcar->rp_irq); +err_remove_domain: +irq_domain_remove (rcar->rp_irq_domain); +err_free_fwnode: +irq_domain_free_fwnode (fwnode); + +return ret; +} + +static void rcar_gen4_pcie_rp_irq_deinit (struct rcar_gen4_pcie *rcar) +{ +struct fwnode_handle *fwnode = rcar->rp_irq_domain->fwnode; + +irq_dispose_mapping (rcar->rp_irq); +irq_domain_remove (rcar->rp_irq_domain); +irq_domain_free_fwnode (fwnode); +} +static int rcar_gen4_pcie_enable_device (struct pci_host_bridge *bridge,struct pci_dev *dev) {@@ -854,6 +1035,13 @@ static int rcar_gen4_pcie_host_setup(struct dw_pcie_rp *pp)if (ret)return ret; +/* +* Root Port MSIs are not delivered. Hide the MSI/MSI-X capabilities +* so port services use the virtual INTx IRQ. +*/ +dw_pcie_remove_capability (dw,PCI_CAP_ID_MSI ); +dw_pcie_remove_capability (dw,PCI_CAP_ID_MSIX ); +msleep (PCIE_T_PVPERL_MS );/* pe_rst requires 100msec delay */ rcar_gen4_pcie_host_perst_assert (pp, false);@@ -869,6 +1057,8 @@ static void rcar_gen4_pcie_quiesce_irqs(struct rcar_gen4_pcie *rcar)/* The MSI status lives in DBI; keep the handler away during the reset. */ disable_irq (rcar->msi_irq); +if (rcar->aer_irq >0 ) +disable_irq (rcar->aer_irq); } static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,@@ -879,6 +1069,8 @@ static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,rcar_gen4_pcie_app_irq_sync_locked (rcar); } +if (rcar->aer_irq >0 ) +enable_irq (rcar->aer_irq);enable_irq (rcar->msi_irq); }@@ -945,6 +1137,8 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)if (pp->bridge) { pp->bridge->enable_device = rcar_gen4_pcie_enable_device; pp->bridge->reset_root_port = rcar_gen4_pcie_reset_root_port; +/* Route Root Port INTx to the virtual IRQ. */ + pp->bridge->map_irq = rcar_gen4_pcie_map_irq; }rcar_gen4_pcie_host_perst_assert (pp, true);@@ -1003,10 +1197,14 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)raw_spin_lock_init (&rcar->app_lock);INIT_WORK (&rcar->link_down_work, rcar_gen4_pcie_link_down_work); - ret =rcar_gen4_pcie_msi_irq_init (rcar); + ret =rcar_gen4_pcie_rp_irq_init (rcar);if (ret)return ret; + ret =rcar_gen4_pcie_msi_irq_init (rcar); +if (ret) + goto err_rp_irq_deinit; + pp->num_vectors =MAX_MSI_IRQS ;/* Reserve an iATU window for the generic PME_Turn_Off implementation. */ pp->use_atu_msg = true;@@ -1018,7 +1216,15 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar) pp->msi_irq[0 ] = -ENODEV ; pp->ops = &rcar_gen4_pcie_host_ops; -return dw_pcie_host_init (pp); + ret =dw_pcie_host_init (pp); +if (ret) + goto err_rp_irq_deinit; + +return 0 ; + +err_rp_irq_deinit: +rcar_gen4_pcie_rp_irq_deinit (rcar); +return ret; }static void rcar_gen4_remove_dw_pcie_rp (struct rcar_gen4_pcie *rcar)@@ -1026,6 +1232,7 @@ static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)/* Cancel recovery work before removing the PCI bus */ rcar_gen4_pcie_rp_events_disarm (rcar);dw_pcie_host_deinit (&rcar->dw.pp); +rcar_gen4_pcie_rp_irq_deinit (rcar); }/* Endpoint mode */ Signature · 2 lines
Best regards, Marek Vasut
Re: [PATCH v5 13/17] PCI: rcar-gen4: Route Root Port AER to a virtual Root Port IRQ
Quoted 58 lines · Koichiro Den
Quoted 46 lines · Marek Vasut
Quoted 37 lines · Koichiro Den
On R-Car Gen4 SoCs, the APP block exposes the Root Port interrupt signals in PCIEINTSTS0 and routes them to platform IRQs: AER to
intreq_pcim_lperr, and PME and bandwidth notifications tointreq_pcim_sub. On an R-Car S4 with GIC ITS, a link retrain set LnkSta.LBMS and latched PCIEINTSTS0[23] (cfg_bw_mgt_msi), but the Root Port's MSI never fired while endpoint MSIs worked fine. With iMSI-RX the Root Port MSI capabilities are already hidden, but its INTx maps tointreq_pcim_sub. Note that even before this driver took over the IRQ withrcar_gen4_pcie_msi_irq_handler, the port service was unable to request it, as the DWC core used that IRQ for its chained MSI handler. In either case, iMSI-RX or an external MSI controller, Root Port interrupts are not delivered to the port services.Hide the Root Port MSI/MSI-X capabilities so the port services use INTx regardless of the MSI controller. Do this in the R-Car host setup, which also runs on reset and resume. Use a virtual IRQ to deliver notifications from separate platform IRQs to the standard handlers. Route AER from the optional "aer" interrupt. DTs without it remain supported, but Root Port AER stays unavailable.
The APP block may still receive MSI-form notifications because hiding the Root Port's MSI capabilities does not clear MSICAP0.MSIE. Handle these alongside the INTx-form level signals. Clear the MSI-form latch before the AER handler clears Root Error Status, so a late APP clear does not erase a new notification.
Check for reset requests before dispatch: on S4, DBI accesses from the port services can hang the host immediately after link down.
The next patch also feeds PME and bandwidth notifications from "msi" into this IRQ, so both parents may dispatch concurrently.
handle_level_irq()callsirq_ack()before checking whether it can run the handlers. Keep the APP latches set while the IRQ is in progress or disabled, so the level parents can retry through the reset checks. This avoids losing notifications without a raw lock around port services that take regular spinlocks.Quoted 7 lines
Signed-off-by Koichiro Den <[email protected]>---
Changes since v4:
- Hide the Root Port MSI/MSI-X capabilities inhost_setup()instead of adding ano_rp_msiflag to the DWC core. (Shawn)
https://lore.kernel.org/r/bi3rvyqpdu4iq6pfyrqvx7eplyim6gmg2ijal67mpateobmtlq@gec7zewfm76r/
I did not carry Marek's Reviewed-by tag over due to the change.Quoted 6 lines
Bjorn/Manivannan, can you please have a look at this patch and the MSI handling, to be sure this is OK ?
(no further comments from me below this point)
Thank you !
Marek, thanks for the review.
Bjorn/Mani, just a gentle ping. Could one of you take a look? To summarize:
Quoted 8 lines
- v4 had '
no_rp_msi' flag to the DWC core. Shawn suggested keeping that in the glue driver instead, which I found reasonable:
https://lore.kernel.org/r/[email protected]/
- v5 hides them inrcar_gen4_pcie_host_setup(). (this patch)If you'd rather I send v6 first, rebased on the
pp->msi_irq_domainrename Shawn and Geert pointed out, please let me know. Otherwise I'll wait for your comments before respinning.
As maintainers are usually super-busy, is there another MSI expert we could ask to offload work from them?
I'd also like to have this series in the next merge window. Thanks everyone for all the work involved, so far!
PATCH v5 14/17 PCI: rcar-gen4: Route Root Port PME and bandwidth notifications
In reply to Koichiro DenThe Root Port's PME and link bandwidth notifications are reported in PCIEINTSTS0 and routed to intreq_pcim_sub, the interrupt shared with the integrated MSI receiver and the reset requests. Dispatch these notifications to the virtual Root Port IRQ from the "msi" IRQ handler after handling any pending iMSI-RX interrupts. This keeps Root Port service handling out of the path between the reset-request check and the iMSI-RX status reads. The existing dispatcher rechecks reset requests before invoking the port services.
Enable the sources only while the Root Port IRQ is unmasked, and clear the MSI-form latches in irq_ack together with the AER one. On an R-Car S4 with the GIC ITS as MSI parent, a notification from a link retrain now reaches the bandwidth controller service, which clears LnkSta.LBMS.
---
Changes since v3:
- No changes.
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 30 ++++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-)
drivers/pci/controller/dwc/pcie-rcar-gen4.c +23 -7
@@ -66,6 +66,16 @@#define AER_RC_ERR_INT BIT(27)#define RP_AER_MASK (AER_RC_ERR_MSI | AER_RC_ERR_INT) +/* Root Port notifications routed to intreq_pcim_sub ("msi") */ +#define BW_MGT_MSI BIT(23) +#define BW_MGT_INT BIT(22) +#define LINK_AUTO_BW_MSI BIT(21) +#define LINK_AUTO_BW_INT BIT(20) +#define PME_MSI BIT(12) +#define PME_INT BIT(10) +#define RP_SUB_MASK (BW_MGT_MSI | BW_MGT_INT | LINK_AUTO_BW_MSI | \ + LINK_AUTO_BW_INT | PME_MSI | PME_INT) +/* PCIe DMA Interrupt Status Enable */ #define PCIEDMAINTSTSEN 0x0314#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)
@@ -75,7 +85,8 @@/* PCIe Interrupt Status 0 Clear */ #define PCIEINTSTS0CLR 0x0340 -#define RP_LATCHED_MASK AER_RC_ERR_MSI +#define RP_LATCHED_MASK (AER_RC_ERR_MSI | BW_MGT_MSI | LINK_AUTO_BW_MSI | \ + PME_MSI)/* Port Logic Registers 89 */ #define PRTLGC89 0x0b70
@@ -136,7 +147,7 @@ struct rcar_gen4_pcie {struct platform_device *pdev;struct reset_control *perst;const struct rcar_gen4_pcie_drvdata *drvdata; -/* intreq_pcim_sub ("msi"): iMSI-RX and other controller notifications */ +/* intreq_pcim_sub ("msi"): iMSI-RX, reset requests, Root Port PME/bandwidth */ int msi_irq;struct work_struct link_down_work;/* Allow Root Port event handling after enumeration and until teardown. */
@@ -579,7 +590,7 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar)lockdep_assert_held (&rcar->app_lock); val =readl (rcar->base +PCIEINTSTS0EN ); - val &= ~(MSI_CTRL_INT |RP_AER_MASK ); + val &= ~(MSI_CTRL_INT |RP_AER_MASK |RP_SUB_MASK );/* * Note that rcar->rp_events_armed is not checked here because device * removal may still need iMSI-RX interrupts after Root Port events have
@@ -589,6 +600,8 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar) val |=MSI_CTRL_INT ;if (rp_events && rcar->rp_irq_enabled && rcar->aer_irq >0 ) val |=RP_AER_MASK ; +if (rp_events && rcar->rp_irq_enabled) + val |=RP_SUB_MASK ;writel (val, rcar->base +PCIEINTSTS0EN ); val =readl (rcar->base +PCIERSTSTSEN );
@@ -736,12 +749,15 @@ static irqreturn_t rcar_gen4_pcie_msi_irq_handler(int irq, void *data) status =readl (rcar->base +PCIEINTSTS0 ) &readl (rcar->base +PCIEINTSTS0EN ); -if (!(status &MSI_CTRL_INT )) -return IRQ_NONE ; +if (status &MSI_CTRL_INT ) +dw_handle_msi_irq (&rcar->dw.pp); +if (status &RP_SUB_MASK ) +rcar_gen4_pcie_rp_irq_dispatch (rcar); -dw_handle_msi_irq (&rcar->dw.pp); +if (status & (MSI_CTRL_INT |RP_SUB_MASK )) +return IRQ_HANDLED ; -return IRQ_HANDLED ; +return IRQ_NONE ; }static int rcar_gen4_pcie_msi_irq_init (struct rcar_gen4_pcie *rcar)
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 14/17] PCI: rcar-gen4: Route Root Port PME and bandwidth notifications
Sashiko has reviewed this patch and found no issues.
Quoted 9 lines
The Root Port's PME and link bandwidth notifications are reported in PCIEINTSTS0 and routed to
intreq_pcim_sub, the interrupt shared with the integrated MSI receiver and the reset requests. Dispatch these notifications to the virtual Root Port IRQ from the "msi" IRQ handler after handling any pending iMSI-RX interrupts. This keeps Root Port service handling out of the path between the reset-request check and the iMSI-RX status reads. The existing dispatcher rechecks reset requests before invoking the port services.
Quoted 7 lines
Enable the sources only while the Root Port IRQ is unmasked, and clear the MSI-form latches in
irq_acktogether with the AER one. On an R-Car S4 with the GIC ITS as MSI parent, a notification from a link retrain now reaches the bandwidth controller service, which clears LnkSta.LBMS.Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14
PATCH v5 15/17 arm64: dts: renesas: r8a779f0: Describe the PCIe AER interrupts
In reply to Koichiro DenThe PCIe controllers report Root Port AER notifications through intreq_pci0_lperr (SPI 421) and intreq_pci1_lperr (SPI 428), respectively, separately from the integrated MSI receiver interrupts. Describe these interrupts with the "aer" name so the controller driver can use them for Root Port error reporting.
Append the new interrupts to preserve the existing interrupt indices.
---
Changes since v3:
- No changes.
arch/arm64/boot/dts/renesas/r8a779f0.dtsi | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/renesas/r8a779f0.dtsi +6 -4
@@ -785,8 +785,9 @@ pciec0: pcie@e65d0000 { interrupts = <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>, - <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "msi", "dma", "sft_ce", "app"; + <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi", "dma", "sft_ce", "app", "aer"; clocks = <&cpg CPG_MOD 624>, <&pcie0_clkref>; clock-names = "core", "ref"; power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
@@ -824,8 +825,9 @@ pciec1: pcie@e65d8000 { interrupts = <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>, - <GIC_SPI 429 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "msi", "dma", "sft_ce", "app"; + <GIC_SPI 429 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 428 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi", "dma", "sft_ce", "app", "aer"; clocks = <&cpg CPG_MOD 625>, <&pcie1_clkref>; clock-names = "core", "ref"; power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 15/17] arm64: dts: renesas: r8a779f0: Describe the PCIe AER interrupts
Sashiko has reviewed this patch and found no issues.
Quoted 10 lines
The PCIe controllers report Root Port AER notifications through
intreq_pci0_lperr(SPI 421) andintreq_pci1_lperr(SPI 428), respectively, separately from the integrated MSI receiver interrupts. Describe these interrupts with the "aer" name so the controller driver can use them for Root Port error reporting.Append the new interrupts to preserve the existing interrupt indices.
Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15
PATCH v5 16/17 arm64: dts: renesas: r8a779g0: Describe the PCIe AER interrupts
In reply to Koichiro DenThe PCIe controllers report Root Port AER notifications through SPI 454 and SPI 461, respectively, separately from the integrated MSI receiver interrupts. Describe these interrupts with the "aer" name so the controller driver can use them for Root Port error reporting.
Append the new interrupts to preserve the existing interrupt indices.
---
Changes since v4:
- Add Marek's Reviewed-by tag. No code changes.
arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/renesas/r8a779g0.dtsi +6 -4
@@ -801,8 +801,9 @@ pciec0: pcie@e65d0000 { interrupts = <GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 450 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 451 IRQ_TYPE_LEVEL_HIGH>, - <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "msi", "dma", "sft_ce", "app"; + <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 454 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi", "dma", "sft_ce", "app", "aer"; clocks = <&cpg CPG_MOD 624>, <&pcie0_clkref>; clock-names = "core", "ref"; power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
@@ -850,8 +851,9 @@ pciec1: pcie@e65d8000 { interrupts = <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>, - <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "msi", "dma", "sft_ce", "app"; + <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi", "dma", "sft_ce", "app", "aer"; clocks = <&cpg CPG_MOD 625>, <&pcie1_clkref>; clock-names = "core", "ref"; power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 16/17] arm64: dts: renesas: r8a779g0: Describe the PCIe AER interrupts
Sashiko has reviewed this patch and found no issues.
Quoted 10 lines
The PCIe controllers report Root Port AER notifications through SPI 454 and SPI 461, respectively, separately from the integrated MSI receiver interrupts. Describe these interrupts with the "aer" name so the controller driver can use them for Root Port error reporting.
Append the new interrupts to preserve the existing interrupt indices.
Suggested-by Marek Vasut <[email protected]>Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16
PATCH v5 17/17 arm64: dts: renesas: r8a779h0: Describe the PCIe AER interrupt
In reply to Koichiro DenThe PCIe controller reports Root Port AER notifications through SPI 454, separately from the integrated MSI receiver interrupt. Describe this interrupt with the "aer" name so the controller driver can use it for Root Port error reporting.
Append the new interrupt to preserve the existing interrupt indices.
---
Changes since v4:
- Add Marek's Reviewed-by tag. No code changes.
arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/renesas/r8a779h0.dtsi +3 -2
@@ -683,8 +683,9 @@ pciec0: pcie@e65d0000 { interrupts = <GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 450 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 451 IRQ_TYPE_LEVEL_HIGH>, - <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "msi", "dma", "sft_ce", "app"; + <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 454 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi", "dma", "sft_ce", "app", "aer"; clocks = <&cpg CPG_MOD 624>, <&pcie0_clkref>; clock-names = "core", "ref"; power-domains = <&sysc R8A779H0_PD_A2PCIPHY>;
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 17/17] arm64: dts: renesas: r8a779h0: Describe the PCIe AER interrupt
Sashiko has reviewed this patch and found no issues.
Quoted 10 lines
The PCIe controller reports Root Port AER notifications through SPI 454, separately from the integrated MSI receiver interrupt. Describe this interrupt with the "aer" name so the controller driver can use it for Root Port error reporting.
Append the new interrupt to preserve the existing interrupt indices.
Suggested-by Marek Vasut <[email protected]>Reviewed-by Marek Vasut <[email protected]>Signed-off-by Koichiro Den <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=17
Hi Den-san,
Thanks for your series!
Quoted 17 lines · Koichiro Den
Proposed merge plan
-------------------With v5, this series seems to have almost converged, so I'd like to ask how maintainers would prefer to merge it. Any advice would be greatly appreciated. Here's what I have in mind:
To Mani:
I'd expect patches 1-14 to go through the PCI tree. One option would be a two-stage approach like:
1. Take patches 1/2/5/10 through controller/dwc ("PCI: dwc:" patches).
2. Once those patches are available in controller/dwc-rcar-gen4, take the remaining patches 3/4/6-9/11-14 there
(the R-Car driver and binding patches).
Quoted 4 lines
To Geert:
Once patch 4 is available in renesas-devel, I'd like to ask you to take patches 15-17 together.
I don't think I have to wait that long, as there is no hard dependency? As soon as the DT bindings (patch 4) are accepted in pci/next, I can queue the DTS patches in renesas-devel?
Thanks!
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 25 lines · Geert Uytterhoeven
Hi Den-san,
Thanks for your series!
Quoted 20 lines · Koichiro Den
Proposed merge plan
-------------------With v5, this series seems to have almost converged, so I'd like to ask how maintainers would prefer to merge it. Any advice would be greatly appreciated. Here's what I have in mind:
To Mani:
I'd expect patches 1-14 to go through the PCI tree. One option would be a two-stage approach like:
1. Take patches 1/2/5/10 through controller/dwc ("PCI: dwc:" patches).
2. Once those patches are available in controller/dwc-rcar-gen4, take the remaining patches 3/4/6-9/11-14 there
(the R-Car driver and binding patches).To Geert:
Once patch 4 is available in renesas-devel, I'd like to ask you to
^^^^^^^^^^^^^(a)
Quoted 5 lines
Quoted 1 line
take patches 15-17 together.
I don't think I have to wait that long, as there is no hard dependency? As soon as the DT bindings (patch 4) are accepted in pci/next, I can queue the DTS patches in renesas-devel?
Hm, you're absolutely right, waiting for patch 4 to be accepted in pci/next is enough. Let me correct that part as you suggest, i.e. s#renesas-devel#pci/next# at (a) above.
Thank you!
Best regards,
Koichiro Den
Quoted 13 lines
Thanks!
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds