All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] PCI: rcar-gen4: Add missing PM ops
@ 2026-09-07 15:36 Marek Vasut
  2026-09-07 15:54 ` sashiko-bot
  2026-09-07 16:05 ` Geert Uytterhoeven
  0 siblings, 2 replies; 3+ messages in thread
From: Marek Vasut @ 2026-09-07 15:36 UTC (permalink / raw)
  To: linux-pci
  Cc: Marek Vasut, Geert Uytterhoeven, Krzysztof Wilczyński,
	Bjorn Helgaas, Conor Dooley, Koichiro Den, Krzysztof Kozlowski,
	Lorenzo Pieralisi, Magnus Damm, Manivannan Sadhasivam,
	Rob Herring, Yoshihiro Shimoda, devicetree, linux-kernel,
	linux-renesas-soc

The R-Car Gen4 PCIe controller is part of a power domain. On R-Car S4
and V4H, this is an always-on power domain which is not shut down in
suspend. On R-Car V4M, this is a dedicated A2PCIPHY power domain,
which is shut down during suspend, and the controller loses state,
which prevents the PCIe from working after resume.

Fix this by adding generic suspend/resume noirq ops for the controller,
which tear the link down on suspend, and restart it on resume. Use the
same PM ops on all of R-Car Gen4 to gracefully suspend and resume the
PCIe link on V4H and S4 too.

Test case which demonstrates the problem on R-Car V4M:
"
$ hexdump -vC /sys/bus/pci/devices/0000:00:00.0/config > /tmp/pre
$ echo s2idle > /sys/power/mem_sleep
$ echo platform > /sys/power/pm_test
$ echo mem > /sys/power/state
$ hexdump -vC /sys/bus/pci/devices/0000:00:00.0/config > /tmp/post
$ diff -Naru /tmp/pre /tmp/post
...
-00000000  12 19 32 00 07 05 10 00  00 00 04 06 00 00 01 00
+00000000  12 19 32 00 07 05 10 00  00 00 00 ff 00 00 80 00
                                          ^^^^^
                                    Class 0604->00ff
"

Fixes: 0d0c551011df ("PCI: rcar-gen4: Add R-Car Gen4 PCIe controller support for host mode")
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Koichiro Den <den@valinux.co.jp>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: - Add TB from Geert for RC mode
    - Use pm_sleep_ptr()
    - Call HOST suspend/resume ops only in case the controller is in RC mode
---
NOTE: Depends on series
      https://lore.kernel.org/linux-pci/20260903205153.283553-1-marek.vasut+renesas@mailbox.org/
---
 drivers/pci/controller/dwc/pcie-rcar-gen4.c | 26 +++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 5690dc32f47e0..7269e08284360 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -909,6 +909,28 @@ static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable
 	return 0;
 }
 
+static int rcar_gen4_pcie_suspend_noirq(struct device *dev)
+{
+	struct rcar_gen4_pcie *rcar = dev_get_drvdata(dev);
+	struct dw_pcie *dw = &rcar->dw;
+
+	if (dw->mode != DW_PCIE_RC_TYPE)
+		return 0;
+
+	return dw_pcie_suspend_noirq(dw);
+}
+
+static int rcar_gen4_pcie_resume_noirq(struct device *dev)
+{
+	struct rcar_gen4_pcie *rcar = dev_get_drvdata(dev);
+	struct dw_pcie *dw = &rcar->dw;
+
+	if (dw->mode != DW_PCIE_RC_TYPE)
+		return 0;
+
+	return dw_pcie_resume_noirq(dw);
+}
+
 static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie = {
 	.ltssm_control = r8a779f0_pcie_ltssm_control,
 	.mode = DW_PCIE_RC_TYPE,
@@ -952,10 +974,14 @@ static const struct of_device_id rcar_gen4_pcie_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, rcar_gen4_pcie_of_match);
 
+DEFINE_NOIRQ_DEV_PM_OPS(rcar_gen4_pcie_pm_ops,
+			rcar_gen4_pcie_suspend_noirq, rcar_gen4_pcie_resume_noirq);
+
 static struct platform_driver rcar_gen4_pcie_driver = {
 	.driver = {
 		.name = "pcie-rcar-gen4",
 		.of_match_table = rcar_gen4_pcie_of_match,
+		.pm = pm_sleep_ptr(&rcar_gen4_pcie_pm_ops),
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 	},
 	.probe = rcar_gen4_pcie_probe,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] PCI: rcar-gen4: Add missing PM ops
  2026-09-07 15:36 [PATCH v2] PCI: rcar-gen4: Add missing PM ops Marek Vasut
@ 2026-09-07 15:54 ` sashiko-bot
  2026-09-07 16:05 ` Geert Uytterhoeven
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-07 15:54 UTC (permalink / raw)
  To: Marek Vasut; +Cc: robh, wsa+renesas, conor+dt, linux-pci, devicetree

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized variable dw->mode causes suspend/resume callbacks to be silently skipped.
- [Low] Missing `static` keyword on `rcar_gen4_pcie_pm_ops` definition.
--

commit 0e99d69c4bfd5a0067af46f15d0ef68cd860b192
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>

PCI: rcar-gen4: Add missing PM ops

The R-Car Gen4 PCIe controller is part of a power domain that is shut down
on V4M during suspend, losing state. This patch adds generic suspend/resume
noirq ops to tear down and restart the link on suspend/resume.

> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index fbe465a29068f..47754aee84278 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c

[ ... ]

> @@ -849,6 +849,28 @@ static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable
>  	return 0;
>  }
>  
> +static int rcar_gen4_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct rcar_gen4_pcie *rcar = dev_get_drvdata(dev);
> +	struct dw_pcie *dw = &rcar->dw;
> +
> +	if (dw->mode != DW_PCIE_RC_TYPE)
> +		return 0;

[Severity: High]
Is dw->mode guaranteed to be initialized here when CONFIG_PCIE_DW_DEBUGFS is
disabled?

It appears dw->mode relies on dwc_pcie_debugfs_init() for initialization.
If debugfs is disabled, dwc_pcie_debugfs_init() becomes an empty inline
function, leaving dw->mode as 0 (DW_PCIE_UNKNOWN_TYPE).

Because 0 != DW_PCIE_RC_TYPE, wouldn't this cause the callbacks to silently
bypass the suspend and resume logic entirely on production builds without
debugfs, leaving the PCIe link up during suspend and dead after resume?

> +
> +	return dw_pcie_suspend_noirq(dw);
> +}
> +
> +static int rcar_gen4_pcie_resume_noirq(struct device *dev)
> +{
> +	struct rcar_gen4_pcie *rcar = dev_get_drvdata(dev);
> +	struct dw_pcie *dw = &rcar->dw;
> +
> +	if (dw->mode != DW_PCIE_RC_TYPE)
> +		return 0;

[Severity: High]
Similar to the suspend callback above, will this also silently skip resuming
the link when debugfs is disabled?

> +
> +	return dw_pcie_resume_noirq(dw);
> +}

[ ... ]

> +DEFINE_NOIRQ_DEV_PM_OPS(rcar_gen4_pcie_pm_ops,
> +			rcar_gen4_pcie_suspend_noirq, rcar_gen4_pcie_resume_noirq);

[Severity: Low]
Should rcar_gen4_pcie_pm_ops be marked static?

The DEFINE_NOIRQ_DEV_PM_OPS macro does not include the static keyword in its
expansion. By not explicitly declaring it static here, this creates a global
symbol that pollutes the global namespace, even though it is only used
locally within this driver file.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907153711.653861-1-marek.vasut+renesas@mailbox.org?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] PCI: rcar-gen4: Add missing PM ops
  2026-09-07 15:36 [PATCH v2] PCI: rcar-gen4: Add missing PM ops Marek Vasut
  2026-09-07 15:54 ` sashiko-bot
@ 2026-09-07 16:05 ` Geert Uytterhoeven
  1 sibling, 0 replies; 3+ messages in thread
From: Geert Uytterhoeven @ 2026-09-07 16:05 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-pci, Krzysztof Wilczyński, Bjorn Helgaas, Conor Dooley,
	Koichiro Den, Krzysztof Kozlowski, Lorenzo Pieralisi, Magnus Damm,
	Manivannan Sadhasivam, Rob Herring, Yoshihiro Shimoda, devicetree,
	linux-kernel, linux-renesas-soc

On Mon, 7 Sept 2026 at 17:37, Marek Vasut
<marek.vasut+renesas@mailbox.org> wrote:
> The R-Car Gen4 PCIe controller is part of a power domain. On R-Car S4
> and V4H, this is an always-on power domain which is not shut down in
> suspend. On R-Car V4M, this is a dedicated A2PCIPHY power domain,
> which is shut down during suspend, and the controller loses state,
> which prevents the PCIe from working after resume.
>
> Fix this by adding generic suspend/resume noirq ops for the controller,
> which tear the link down on suspend, and restart it on resume. Use the
> same PM ops on all of R-Car Gen4 to gracefully suspend and resume the
> PCIe link on V4H and S4 too.
>
> Test case which demonstrates the problem on R-Car V4M:
> "
> $ hexdump -vC /sys/bus/pci/devices/0000:00:00.0/config > /tmp/pre
> $ echo s2idle > /sys/power/mem_sleep
> $ echo platform > /sys/power/pm_test
> $ echo mem > /sys/power/state
> $ hexdump -vC /sys/bus/pci/devices/0000:00:00.0/config > /tmp/post
> $ diff -Naru /tmp/pre /tmp/post
> ...
> -00000000  12 19 32 00 07 05 10 00  00 00 04 06 00 00 01 00
> +00000000  12 19 32 00 07 05 10 00  00 00 00 ff 00 00 80 00
>                                           ^^^^^
>                                     Class 0604->00ff
> "
>
> Fixes: 0d0c551011df ("PCI: rcar-gen4: Add R-Car Gen4 PCIe controller support for host mode")
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> V2: - Add TB from Geert for RC mode
>     - Use pm_sleep_ptr()
>     - Call HOST suspend/resume ops only in case the controller is in RC mode

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-07 16:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07 15:36 [PATCH v2] PCI: rcar-gen4: Add missing PM ops Marek Vasut
2026-09-07 15:54 ` sashiko-bot
2026-09-07 16:05 ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.