[PATCH] PCI: starfive: Enable PCIe controller's PM runtime before probing host bridge

Mayank Rana posted 1 patch 1 month, 2 weeks ago
drivers/pci/controller/plda/pcie-starfive.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
[PATCH] PCI: starfive: Enable PCIe controller's PM runtime before probing host bridge
Posted by Mayank Rana 1 month, 2 weeks ago
Commit 02787a3b4d10 ("PCI/PM: Enable runtime power management for host
bridges") enables runtime PM for host bridge enforcing dependency chain
between PCIe controller, host bridge and endpoint devices. With this,
Starfive PCIe controller driver's probe enables host bridge (child device)
PM runtime before parent's PM runtime (Starfive PCIe controller device)
causing below warning and callstack:

pcie-starfive 940000000.pcie: Enabling runtime PM for inactive device
with active children

======================================================
WARNING: possible circular locking dependency detected
6.12.0-rc1+ #15438 Not tainted
------------------------------------------------------
systemd-udevd/159 is trying to acquire lock:
ffffffff81822520 (console_owner){-.-.}-{0:0}, at:
console_lock_spinning_enable+0x3a/0x60

but task is already holding lock:
ffffffd6c0b3d980 (&dev->power.lock){-...}-{2:2}, at:
pm_runtime_enable+0x1e/0xb6

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #2 (&dev->power.lock){-...}-{2:2}:
        lock_acquire.part.0+0xa2/0x1d4
        lock_acquire+0x44/0x5a
        _raw_spin_lock_irqsave+0x3a/0x64
        __pm_runtime_resume+0x40/0x86
        __uart_start+0x40/0xb2
        uart_write+0x90/0x220
        n_tty_write+0x10a/0x40e
        file_tty_write.constprop.0+0x10c/0x230
        redirected_tty_write+0x84/0xbc
        do_iter_readv_writev+0x100/0x166
        vfs_writev+0xc6/0x398
        do_writev+0x5c/0xca
        __riscv_sys_writev+0x16/0x1e
        do_trap_ecall_u+0x1b6/0x1e2
        _new_vmalloc_restore_context_a0+0xc2/0xce

-> #1 (&port_lock_key){-.-.}-{2:2}:
        lock_acquire.part.0+0xa2/0x1d4
        lock_acquire+0x44/0x5a
        _raw_spin_lock_irqsave+0x3a/0x64
        serial8250_console_write+0x2a0/0x474
        univ8250_console_write+0x22/0x2a
        console_flush_all+0x2f6/0x3c8
        console_unlock+0x80/0x1a8
        vprintk_emit+0x10e/0x2e0
        vprintk_default+0x16/0x1e
        vprintk+0x1e/0x3c
        _printk+0x36/0x50
        register_console+0x292/0x418
        serial_core_register_port+0x6d6/0x6dc
        serial_ctrl_register_port+0xc/0x14
        uart_add_one_port+0xc/0x14
        serial8250_register_8250_port+0x288/0x428
        dw8250_probe+0x422/0x518
        platform_probe+0x4e/0x92
        really_probe+0x10a/0x2da
        __driver_probe_device.part.0+0xb2/0xe8
        driver_probe_device+0x78/0xc4
        __device_attach_driver+0x66/0xc6
        bus_for_each_drv+0x5c/0xb0
        __device_attach+0x84/0x13c
        device_initial_probe+0xe/0x16
        bus_probe_device+0x88/0x8a
        deferred_probe_work_func+0xd4/0xee
        process_one_work+0x1e0/0x534
        worker_thread+0x166/0x2cc
        kthread+0xc4/0xe0
        ret_from_fork+0xe/0x18

-> #0 (console_owner){-.-.}-{0:0}:
        check_noncircular+0x10e/0x122
        __lock_acquire+0x105c/0x1f4a
        lock_acquire.part.0+0xa2/0x1d4
        lock_acquire+0x44/0x5a
        console_lock_spinning_enable+0x58/0x60
        console_flush_all+0x2cc/0x3c8
        console_unlock+0x80/0x1a8
        vprintk_emit+0x10e/0x2e0
        dev_vprintk_emit+0xea/0x112
        dev_printk_emit+0x2e/0x48
        __dev_printk+0x40/0x5c
        _dev_warn+0x46/0x60
        pm_runtime_enable+0x98/0xb6
        starfive_pcie_probe+0x12e/0x228 [pcie_starfive]
        platform_probe+0x4e/0x92
        really_probe+0x10a/0x2da
        __driver_probe_device.part.0+0xb2/0xe8
        driver_probe_device+0x78/0xc4
        __driver_attach+0x54/0x162
        bus_for_each_dev+0x58/0xa4
        driver_attach+0x1a/0x22
        bus_add_driver+0xec/0x1ce
        driver_register+0x3e/0xd8
        __platform_driver_register+0x1c/0x24
        starfive_pcie_driver_init+0x20/0x1000 [pcie_starfive]
        do_one_initcall+0x5e/0x28c
        do_init_module+0x52/0x1ba
        load_module+0x1440/0x18f0
        init_module_from_file+0x76/0xae
        idempotent_init_module+0x18c/0x24a
        __riscv_sys_finit_module+0x52/0x82
        do_trap_ecall_u+0x1b6/0x1e2
        _new_vmalloc_restore_context_a0+0xc2/0xce

other info that might help us debug this:

Chain exists of:
   console_owner --> &port_lock_key --> &dev->power.lock

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(&dev->power.lock);
                                lock(&port_lock_key);
                                lock(&dev->power.lock);
   lock(console_owner);

  *** DEADLOCK ***

4 locks held by systemd-udevd/159:
  #0: ffffffd6c0b3d8f8 (&dev->mutex){....}-{3:3}, at:
__driver_attach+0x4c/0x162
  #1: ffffffd6c0b3d980 (&dev->power.lock){-...}-{2:2}, at:
pm_runtime_enable+0x1e/0xb6
  #2: ffffffff818223b0 (console_lock){+.+.}-{0:0}, at:
dev_vprintk_emit+0xea/0x112
  #3: ffffffff81822448 (console_srcu){....}-{0:0}, at:
console_flush_all+0x4e/0x3c8

stack backtrace:
CPU: 1 UID: 0 PID: 159 Comm: systemd-udevd Not tainted 6.12.0-rc1+ #15438
Hardware name: StarFive VisionFive 2 v1.2A (DT)
Call Trace:
[<ffffffff80006a02>] dump_backtrace+0x1c/0x24
[<ffffffff80b70b3e>] show_stack+0x2c/0x38
[<ffffffff80b7f8f8>] dump_stack_lvl+0x7a/0xb4
[<ffffffff80b7f946>] dump_stack+0x14/0x1c
[<ffffffff8007fbc2>] print_circular_bug+0x2aa/0x350
[<ffffffff8007fd76>] check_noncircular+0x10e/0x122
[<ffffffff80082a3c>] __lock_acquire+0x105c/0x1f4a
[<ffffffff80084148>] lock_acquire.part.0+0xa2/0x1d4
[<ffffffff800842be>] lock_acquire+0x44/0x5a
[<ffffffff8008b3e8>] console_lock_spinning_enable+0x58/0x60
[<ffffffff8008c0c2>] console_flush_all+0x2cc/0x3c8
[<ffffffff8008c23e>] console_unlock+0x80/0x1a8
[<ffffffff8008c710>] vprintk_emit+0x10e/0x2e0
[<ffffffff80b79ec8>] dev_vprintk_emit+0xea/0x112
[<ffffffff80b79f1e>] dev_printk_emit+0x2e/0x48
[<ffffffff80b7a006>] __dev_printk+0x40/0x5c
[<ffffffff80b7a2be>] _dev_warn+0x46/0x60
[<ffffffff807037ae>] pm_runtime_enable+0x98/0xb6
[<ffffffff02763240>] starfive_pcie_probe+0x12e/0x228 [pcie_starfive]
[<ffffffff806f83f6>] platform_probe+0x4e/0x92
[<ffffffff80b7a680>] really_probe+0x10a/0x2da
[<ffffffff80b7a902>] __driver_probe_device.part.0+0xb2/0xe8
[<ffffffff806f6112>] driver_probe_device+0x78/0xc4
[<ffffffff806f6278>] __driver_attach+0x54/0x162
[<ffffffff806f42e6>] bus_for_each_dev+0x58/0xa4
[<ffffffff806f5c9e>] driver_attach+0x1a/0x22
[<ffffffff806f54ce>] bus_add_driver+0xec/0x1ce
[<ffffffff806f7112>] driver_register+0x3e/0xd8
[<ffffffff806f80cc>] __platform_driver_register+0x1c/0x24
[<ffffffff027ea020>] starfive_pcie_driver_init+0x20/0x1000 [pcie_starfive]
[<ffffffff800027ba>] do_one_initcall+0x5e/0x28c
[<ffffffff800bb4d8>] do_init_module+0x52/0x1ba
[<ffffffff800bcc8a>] load_module+0x1440/0x18f0
[<ffffffff800bd2f0>] init_module_from_file+0x76/0xae
[<ffffffff800bd4b4>] idempotent_init_module+0x18c/0x24a
[<ffffffff800bd5fc>] __riscv_sys_finit_module+0x52/0x82
[<ffffffff80b804a0>] do_trap_ecall_u+0x1b6/0x1e2
[<ffffffff80b8c536>] _new_vmalloc_restore_context_a0+0xc2/0xce

Fix this issue by enabling starfive pcie controller device's PM runtime
status before calling into pci_host_probe() through plda_pcie_host_init().

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Fixes: 02787a3b4d10 ("PCI/PM: Enable runtime power management for host bridges")
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
 drivers/pci/controller/plda/pcie-starfive.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
index 0567ec373a3e..e73c1b7bc8ef 100644
--- a/drivers/pci/controller/plda/pcie-starfive.c
+++ b/drivers/pci/controller/plda/pcie-starfive.c
@@ -404,6 +404,9 @@ static int starfive_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_get_sync(&pdev->dev);
+
 	plda->host_ops = &sf_host_ops;
 	plda->num_events = PLDA_MAX_EVENT_NUM;
 	/* mask doorbell event */
@@ -413,11 +416,12 @@ static int starfive_pcie_probe(struct platform_device *pdev)
 	plda->events_bitmap <<= PLDA_NUM_DMA_EVENTS;
 	ret = plda_pcie_host_init(&pcie->plda, &starfive_pcie_ops,
 				  &stf_pcie_event);
-	if (ret)
+	if (ret) {
+		pm_runtime_put_sync(&pdev->dev);
+		pm_runtime_disable(&pdev->dev);
 		return ret;
+	}
 
-	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
 	platform_set_drvdata(pdev, pcie);
 
 	return 0;
-- 
2.25.1
Re: [PATCH] PCI: starfive: Enable PCIe controller's PM runtime before probing host bridge
Posted by Bjorn Helgaas 1 month, 2 weeks ago
On Thu, Oct 10, 2024 at 01:29:50PM -0700, Mayank Rana wrote:
> Commit 02787a3b4d10 ("PCI/PM: Enable runtime power management for host
> bridges") enables runtime PM for host bridge enforcing dependency chain
> between PCIe controller, host bridge and endpoint devices. With this,
> Starfive PCIe controller driver's probe enables host bridge (child device)
> PM runtime before parent's PM runtime (Starfive PCIe controller device)
> causing below warning and callstack:

I don't want the bisection hole that would result if we kept
02787a3b4d10 ("PCI/PM: Enable runtime power management for host
bridges") and applied this patch on top of it.

If this is the fix, we'll apply it *first*, followed by 02787a3b4d10
(which will obviously become a different commit), so the locking
problem below described below should never exist in -next or the
upstream tree.

So we need to audit other drivers to make sure they don't have the
same problem as starfive, then make a series with this patch and any
others that need similar fixes, followed by the "PCI/PM: Enable
runtime power management for host bridges" patch.

Presumably this patch fixes a problem all by itself, even without
considering 02787a3b4d10, so the commit message should describe that
problem.

> pcie-starfive 940000000.pcie: Enabling runtime PM for inactive device
> with active children
> 
> ======================================================
> WARNING: possible circular locking dependency detected
> 6.12.0-rc1+ #15438 Not tainted
> ------------------------------------------------------
> systemd-udevd/159 is trying to acquire lock:
> ffffffff81822520 (console_owner){-.-.}-{0:0}, at:
> console_lock_spinning_enable+0x3a/0x60
> 
> but task is already holding lock:
> ffffffd6c0b3d980 (&dev->power.lock){-...}-{2:2}, at:
> pm_runtime_enable+0x1e/0xb6
> 
> which lock already depends on the new lock.
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #2 (&dev->power.lock){-...}-{2:2}:
>         lock_acquire.part.0+0xa2/0x1d4
>         lock_acquire+0x44/0x5a
>         _raw_spin_lock_irqsave+0x3a/0x64
>         __pm_runtime_resume+0x40/0x86
>         __uart_start+0x40/0xb2
>         uart_write+0x90/0x220
>         n_tty_write+0x10a/0x40e
>         file_tty_write.constprop.0+0x10c/0x230
>         redirected_tty_write+0x84/0xbc
>         do_iter_readv_writev+0x100/0x166
>         vfs_writev+0xc6/0x398
>         do_writev+0x5c/0xca
>         __riscv_sys_writev+0x16/0x1e
>         do_trap_ecall_u+0x1b6/0x1e2
>         _new_vmalloc_restore_context_a0+0xc2/0xce
> 
> -> #1 (&port_lock_key){-.-.}-{2:2}:
>         lock_acquire.part.0+0xa2/0x1d4
>         lock_acquire+0x44/0x5a
>         _raw_spin_lock_irqsave+0x3a/0x64
>         serial8250_console_write+0x2a0/0x474
>         univ8250_console_write+0x22/0x2a
>         console_flush_all+0x2f6/0x3c8
>         console_unlock+0x80/0x1a8
>         vprintk_emit+0x10e/0x2e0
>         vprintk_default+0x16/0x1e
>         vprintk+0x1e/0x3c
>         _printk+0x36/0x50
>         register_console+0x292/0x418
>         serial_core_register_port+0x6d6/0x6dc
>         serial_ctrl_register_port+0xc/0x14
>         uart_add_one_port+0xc/0x14
>         serial8250_register_8250_port+0x288/0x428
>         dw8250_probe+0x422/0x518
>         platform_probe+0x4e/0x92
>         really_probe+0x10a/0x2da
>         __driver_probe_device.part.0+0xb2/0xe8
>         driver_probe_device+0x78/0xc4
>         __device_attach_driver+0x66/0xc6
>         bus_for_each_drv+0x5c/0xb0
>         __device_attach+0x84/0x13c
>         device_initial_probe+0xe/0x16
>         bus_probe_device+0x88/0x8a
>         deferred_probe_work_func+0xd4/0xee
>         process_one_work+0x1e0/0x534
>         worker_thread+0x166/0x2cc
>         kthread+0xc4/0xe0
>         ret_from_fork+0xe/0x18
> 
> -> #0 (console_owner){-.-.}-{0:0}:
>         check_noncircular+0x10e/0x122
>         __lock_acquire+0x105c/0x1f4a
>         lock_acquire.part.0+0xa2/0x1d4
>         lock_acquire+0x44/0x5a
>         console_lock_spinning_enable+0x58/0x60
>         console_flush_all+0x2cc/0x3c8
>         console_unlock+0x80/0x1a8
>         vprintk_emit+0x10e/0x2e0
>         dev_vprintk_emit+0xea/0x112
>         dev_printk_emit+0x2e/0x48
>         __dev_printk+0x40/0x5c
>         _dev_warn+0x46/0x60
>         pm_runtime_enable+0x98/0xb6
>         starfive_pcie_probe+0x12e/0x228 [pcie_starfive]
>         platform_probe+0x4e/0x92
>         really_probe+0x10a/0x2da
>         __driver_probe_device.part.0+0xb2/0xe8
>         driver_probe_device+0x78/0xc4
>         __driver_attach+0x54/0x162
>         bus_for_each_dev+0x58/0xa4
>         driver_attach+0x1a/0x22
>         bus_add_driver+0xec/0x1ce
>         driver_register+0x3e/0xd8
>         __platform_driver_register+0x1c/0x24
>         starfive_pcie_driver_init+0x20/0x1000 [pcie_starfive]
>         do_one_initcall+0x5e/0x28c
>         do_init_module+0x52/0x1ba
>         load_module+0x1440/0x18f0
>         init_module_from_file+0x76/0xae
>         idempotent_init_module+0x18c/0x24a
>         __riscv_sys_finit_module+0x52/0x82
>         do_trap_ecall_u+0x1b6/0x1e2
>         _new_vmalloc_restore_context_a0+0xc2/0xce
> 
> other info that might help us debug this:
> 
> Chain exists of:
>    console_owner --> &port_lock_key --> &dev->power.lock
> 
>   Possible unsafe locking scenario:
> 
>         CPU0                    CPU1
>         ----                    ----
>    lock(&dev->power.lock);
>                                 lock(&port_lock_key);
>                                 lock(&dev->power.lock);
>    lock(console_owner);
> 
>   *** DEADLOCK ***
> 
> 4 locks held by systemd-udevd/159:
>   #0: ffffffd6c0b3d8f8 (&dev->mutex){....}-{3:3}, at:
> __driver_attach+0x4c/0x162
>   #1: ffffffd6c0b3d980 (&dev->power.lock){-...}-{2:2}, at:
> pm_runtime_enable+0x1e/0xb6
>   #2: ffffffff818223b0 (console_lock){+.+.}-{0:0}, at:
> dev_vprintk_emit+0xea/0x112
>   #3: ffffffff81822448 (console_srcu){....}-{0:0}, at:
> console_flush_all+0x4e/0x3c8
> 
> stack backtrace:
> CPU: 1 UID: 0 PID: 159 Comm: systemd-udevd Not tainted 6.12.0-rc1+ #15438
> Hardware name: StarFive VisionFive 2 v1.2A (DT)
> Call Trace:
> [<ffffffff80006a02>] dump_backtrace+0x1c/0x24
> [<ffffffff80b70b3e>] show_stack+0x2c/0x38
> [<ffffffff80b7f8f8>] dump_stack_lvl+0x7a/0xb4
> [<ffffffff80b7f946>] dump_stack+0x14/0x1c
> [<ffffffff8007fbc2>] print_circular_bug+0x2aa/0x350
> [<ffffffff8007fd76>] check_noncircular+0x10e/0x122
> [<ffffffff80082a3c>] __lock_acquire+0x105c/0x1f4a
> [<ffffffff80084148>] lock_acquire.part.0+0xa2/0x1d4
> [<ffffffff800842be>] lock_acquire+0x44/0x5a
> [<ffffffff8008b3e8>] console_lock_spinning_enable+0x58/0x60
> [<ffffffff8008c0c2>] console_flush_all+0x2cc/0x3c8
> [<ffffffff8008c23e>] console_unlock+0x80/0x1a8
> [<ffffffff8008c710>] vprintk_emit+0x10e/0x2e0
> [<ffffffff80b79ec8>] dev_vprintk_emit+0xea/0x112
> [<ffffffff80b79f1e>] dev_printk_emit+0x2e/0x48
> [<ffffffff80b7a006>] __dev_printk+0x40/0x5c
> [<ffffffff80b7a2be>] _dev_warn+0x46/0x60
> [<ffffffff807037ae>] pm_runtime_enable+0x98/0xb6
> [<ffffffff02763240>] starfive_pcie_probe+0x12e/0x228 [pcie_starfive]
> [<ffffffff806f83f6>] platform_probe+0x4e/0x92
> [<ffffffff80b7a680>] really_probe+0x10a/0x2da
> [<ffffffff80b7a902>] __driver_probe_device.part.0+0xb2/0xe8
> [<ffffffff806f6112>] driver_probe_device+0x78/0xc4
> [<ffffffff806f6278>] __driver_attach+0x54/0x162
> [<ffffffff806f42e6>] bus_for_each_dev+0x58/0xa4
> [<ffffffff806f5c9e>] driver_attach+0x1a/0x22
> [<ffffffff806f54ce>] bus_add_driver+0xec/0x1ce
> [<ffffffff806f7112>] driver_register+0x3e/0xd8
> [<ffffffff806f80cc>] __platform_driver_register+0x1c/0x24
> [<ffffffff027ea020>] starfive_pcie_driver_init+0x20/0x1000 [pcie_starfive]
> [<ffffffff800027ba>] do_one_initcall+0x5e/0x28c
> [<ffffffff800bb4d8>] do_init_module+0x52/0x1ba
> [<ffffffff800bcc8a>] load_module+0x1440/0x18f0
> [<ffffffff800bd2f0>] init_module_from_file+0x76/0xae
> [<ffffffff800bd4b4>] idempotent_init_module+0x18c/0x24a
> [<ffffffff800bd5fc>] __riscv_sys_finit_module+0x52/0x82
> [<ffffffff80b804a0>] do_trap_ecall_u+0x1b6/0x1e2
> [<ffffffff80b8c536>] _new_vmalloc_restore_context_a0+0xc2/0xce
> 
> Fix this issue by enabling starfive pcie controller device's PM runtime
> status before calling into pci_host_probe() through plda_pcie_host_init().
> 
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Fixes: 02787a3b4d10 ("PCI/PM: Enable runtime power management for host bridges")
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
>  drivers/pci/controller/plda/pcie-starfive.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
> index 0567ec373a3e..e73c1b7bc8ef 100644
> --- a/drivers/pci/controller/plda/pcie-starfive.c
> +++ b/drivers/pci/controller/plda/pcie-starfive.c
> @@ -404,6 +404,9 @@ static int starfive_pcie_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_get_sync(&pdev->dev);
> +
>  	plda->host_ops = &sf_host_ops;
>  	plda->num_events = PLDA_MAX_EVENT_NUM;
>  	/* mask doorbell event */
> @@ -413,11 +416,12 @@ static int starfive_pcie_probe(struct platform_device *pdev)
>  	plda->events_bitmap <<= PLDA_NUM_DMA_EVENTS;
>  	ret = plda_pcie_host_init(&pcie->plda, &starfive_pcie_ops,
>  				  &stf_pcie_event);
> -	if (ret)
> +	if (ret) {
> +		pm_runtime_put_sync(&pdev->dev);
> +		pm_runtime_disable(&pdev->dev);
>  		return ret;
> +	}
>  
> -	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_get_sync(&pdev->dev);
>  	platform_set_drvdata(pdev, pcie);
>  
>  	return 0;
> -- 
> 2.25.1
>
Re: [PATCH] PCI: starfive: Enable PCIe controller's PM runtime before probing host bridge
Posted by Krishna Chaitanya Chundru 1 month, 2 weeks ago

On 10/11/2024 2:22 AM, Bjorn Helgaas wrote:
> On Thu, Oct 10, 2024 at 01:29:50PM -0700, Mayank Rana wrote:
>> Commit 02787a3b4d10 ("PCI/PM: Enable runtime power management for host
>> bridges") enables runtime PM for host bridge enforcing dependency chain
>> between PCIe controller, host bridge and endpoint devices. With this,
>> Starfive PCIe controller driver's probe enables host bridge (child device)
>> PM runtime before parent's PM runtime (Starfive PCIe controller device)
>> causing below warning and callstack:
> 
> I don't want the bisection hole that would result if we kept
> 02787a3b4d10 ("PCI/PM: Enable runtime power management for host
> bridges") and applied this patch on top of it.
> 
> If this is the fix, we'll apply it *first*, followed by 02787a3b4d10
> (which will obviously become a different commit), so the locking
> problem below described below should never exist in -next or the
> upstream tree.
> 
> So we need to audit other drivers to make sure they don't have theBjorn, I have checked all the drivers in the controller folder where
they are using pm_runtime_enable(), this is the only driver which needs
to be fixed. once this patched was taken can we take "PCI/PM: Enable
  runtime power management for host bridges"

- Krishna Chaitanya.
> same problem as starfive, then make a series with this patch and any
> others that need similar fixes, followed by the "PCI/PM: Enable
> runtime power management for host bridges" patch.
> 
> Presumably this patch fixes a problem all by itself, even without
> considering 02787a3b4d10, so the commit message should describe that
> problem.
> 
>> pcie-starfive 940000000.pcie: Enabling runtime PM for inactive device
>> with active children
>>
>> ======================================================
>> WARNING: possible circular locking dependency detected
>> 6.12.0-rc1+ #15438 Not tainted
>> ------------------------------------------------------
>> systemd-udevd/159 is trying to acquire lock:
>> ffffffff81822520 (console_owner){-.-.}-{0:0}, at:
>> console_lock_spinning_enable+0x3a/0x60
>>
>> but task is already holding lock:
>> ffffffd6c0b3d980 (&dev->power.lock){-...}-{2:2}, at:
>> pm_runtime_enable+0x1e/0xb6
>>
>> which lock already depends on the new lock.
>>
>> the existing dependency chain (in reverse order) is:
>>
>> -> #2 (&dev->power.lock){-...}-{2:2}:
>>          lock_acquire.part.0+0xa2/0x1d4
>>          lock_acquire+0x44/0x5a
>>          _raw_spin_lock_irqsave+0x3a/0x64
>>          __pm_runtime_resume+0x40/0x86
>>          __uart_start+0x40/0xb2
>>          uart_write+0x90/0x220
>>          n_tty_write+0x10a/0x40e
>>          file_tty_write.constprop.0+0x10c/0x230
>>          redirected_tty_write+0x84/0xbc
>>          do_iter_readv_writev+0x100/0x166
>>          vfs_writev+0xc6/0x398
>>          do_writev+0x5c/0xca
>>          __riscv_sys_writev+0x16/0x1e
>>          do_trap_ecall_u+0x1b6/0x1e2
>>          _new_vmalloc_restore_context_a0+0xc2/0xce
>>
>> -> #1 (&port_lock_key){-.-.}-{2:2}:
>>          lock_acquire.part.0+0xa2/0x1d4
>>          lock_acquire+0x44/0x5a
>>          _raw_spin_lock_irqsave+0x3a/0x64
>>          serial8250_console_write+0x2a0/0x474
>>          univ8250_console_write+0x22/0x2a
>>          console_flush_all+0x2f6/0x3c8
>>          console_unlock+0x80/0x1a8
>>          vprintk_emit+0x10e/0x2e0
>>          vprintk_default+0x16/0x1e
>>          vprintk+0x1e/0x3c
>>          _printk+0x36/0x50
>>          register_console+0x292/0x418
>>          serial_core_register_port+0x6d6/0x6dc
>>          serial_ctrl_register_port+0xc/0x14
>>          uart_add_one_port+0xc/0x14
>>          serial8250_register_8250_port+0x288/0x428
>>          dw8250_probe+0x422/0x518
>>          platform_probe+0x4e/0x92
>>          really_probe+0x10a/0x2da
>>          __driver_probe_device.part.0+0xb2/0xe8
>>          driver_probe_device+0x78/0xc4
>>          __device_attach_driver+0x66/0xc6
>>          bus_for_each_drv+0x5c/0xb0
>>          __device_attach+0x84/0x13c
>>          device_initial_probe+0xe/0x16
>>          bus_probe_device+0x88/0x8a
>>          deferred_probe_work_func+0xd4/0xee
>>          process_one_work+0x1e0/0x534
>>          worker_thread+0x166/0x2cc
>>          kthread+0xc4/0xe0
>>          ret_from_fork+0xe/0x18
>>
>> -> #0 (console_owner){-.-.}-{0:0}:
>>          check_noncircular+0x10e/0x122
>>          __lock_acquire+0x105c/0x1f4a
>>          lock_acquire.part.0+0xa2/0x1d4
>>          lock_acquire+0x44/0x5a
>>          console_lock_spinning_enable+0x58/0x60
>>          console_flush_all+0x2cc/0x3c8
>>          console_unlock+0x80/0x1a8
>>          vprintk_emit+0x10e/0x2e0
>>          dev_vprintk_emit+0xea/0x112
>>          dev_printk_emit+0x2e/0x48
>>          __dev_printk+0x40/0x5c
>>          _dev_warn+0x46/0x60
>>          pm_runtime_enable+0x98/0xb6
>>          starfive_pcie_probe+0x12e/0x228 [pcie_starfive]
>>          platform_probe+0x4e/0x92
>>          really_probe+0x10a/0x2da
>>          __driver_probe_device.part.0+0xb2/0xe8
>>          driver_probe_device+0x78/0xc4
>>          __driver_attach+0x54/0x162
>>          bus_for_each_dev+0x58/0xa4
>>          driver_attach+0x1a/0x22
>>          bus_add_driver+0xec/0x1ce
>>          driver_register+0x3e/0xd8
>>          __platform_driver_register+0x1c/0x24
>>          starfive_pcie_driver_init+0x20/0x1000 [pcie_starfive]
>>          do_one_initcall+0x5e/0x28c
>>          do_init_module+0x52/0x1ba
>>          load_module+0x1440/0x18f0
>>          init_module_from_file+0x76/0xae
>>          idempotent_init_module+0x18c/0x24a
>>          __riscv_sys_finit_module+0x52/0x82
>>          do_trap_ecall_u+0x1b6/0x1e2
>>          _new_vmalloc_restore_context_a0+0xc2/0xce
>>
>> other info that might help us debug this:
>>
>> Chain exists of:
>>     console_owner --> &port_lock_key --> &dev->power.lock
>>
>>    Possible unsafe locking scenario:
>>
>>          CPU0                    CPU1
>>          ----                    ----
>>     lock(&dev->power.lock);
>>                                  lock(&port_lock_key);
>>                                  lock(&dev->power.lock);
>>     lock(console_owner);
>>
>>    *** DEADLOCK ***
>>
>> 4 locks held by systemd-udevd/159:
>>    #0: ffffffd6c0b3d8f8 (&dev->mutex){....}-{3:3}, at:
>> __driver_attach+0x4c/0x162
>>    #1: ffffffd6c0b3d980 (&dev->power.lock){-...}-{2:2}, at:
>> pm_runtime_enable+0x1e/0xb6
>>    #2: ffffffff818223b0 (console_lock){+.+.}-{0:0}, at:
>> dev_vprintk_emit+0xea/0x112
>>    #3: ffffffff81822448 (console_srcu){....}-{0:0}, at:
>> console_flush_all+0x4e/0x3c8
>>
>> stack backtrace:
>> CPU: 1 UID: 0 PID: 159 Comm: systemd-udevd Not tainted 6.12.0-rc1+ #15438
>> Hardware name: StarFive VisionFive 2 v1.2A (DT)
>> Call Trace:
>> [<ffffffff80006a02>] dump_backtrace+0x1c/0x24
>> [<ffffffff80b70b3e>] show_stack+0x2c/0x38
>> [<ffffffff80b7f8f8>] dump_stack_lvl+0x7a/0xb4
>> [<ffffffff80b7f946>] dump_stack+0x14/0x1c
>> [<ffffffff8007fbc2>] print_circular_bug+0x2aa/0x350
>> [<ffffffff8007fd76>] check_noncircular+0x10e/0x122
>> [<ffffffff80082a3c>] __lock_acquire+0x105c/0x1f4a
>> [<ffffffff80084148>] lock_acquire.part.0+0xa2/0x1d4
>> [<ffffffff800842be>] lock_acquire+0x44/0x5a
>> [<ffffffff8008b3e8>] console_lock_spinning_enable+0x58/0x60
>> [<ffffffff8008c0c2>] console_flush_all+0x2cc/0x3c8
>> [<ffffffff8008c23e>] console_unlock+0x80/0x1a8
>> [<ffffffff8008c710>] vprintk_emit+0x10e/0x2e0
>> [<ffffffff80b79ec8>] dev_vprintk_emit+0xea/0x112
>> [<ffffffff80b79f1e>] dev_printk_emit+0x2e/0x48
>> [<ffffffff80b7a006>] __dev_printk+0x40/0x5c
>> [<ffffffff80b7a2be>] _dev_warn+0x46/0x60
>> [<ffffffff807037ae>] pm_runtime_enable+0x98/0xb6
>> [<ffffffff02763240>] starfive_pcie_probe+0x12e/0x228 [pcie_starfive]
>> [<ffffffff806f83f6>] platform_probe+0x4e/0x92
>> [<ffffffff80b7a680>] really_probe+0x10a/0x2da
>> [<ffffffff80b7a902>] __driver_probe_device.part.0+0xb2/0xe8
>> [<ffffffff806f6112>] driver_probe_device+0x78/0xc4
>> [<ffffffff806f6278>] __driver_attach+0x54/0x162
>> [<ffffffff806f42e6>] bus_for_each_dev+0x58/0xa4
>> [<ffffffff806f5c9e>] driver_attach+0x1a/0x22
>> [<ffffffff806f54ce>] bus_add_driver+0xec/0x1ce
>> [<ffffffff806f7112>] driver_register+0x3e/0xd8
>> [<ffffffff806f80cc>] __platform_driver_register+0x1c/0x24
>> [<ffffffff027ea020>] starfive_pcie_driver_init+0x20/0x1000 [pcie_starfive]
>> [<ffffffff800027ba>] do_one_initcall+0x5e/0x28c
>> [<ffffffff800bb4d8>] do_init_module+0x52/0x1ba
>> [<ffffffff800bcc8a>] load_module+0x1440/0x18f0
>> [<ffffffff800bd2f0>] init_module_from_file+0x76/0xae
>> [<ffffffff800bd4b4>] idempotent_init_module+0x18c/0x24a
>> [<ffffffff800bd5fc>] __riscv_sys_finit_module+0x52/0x82
>> [<ffffffff80b804a0>] do_trap_ecall_u+0x1b6/0x1e2
>> [<ffffffff80b8c536>] _new_vmalloc_restore_context_a0+0xc2/0xce
>>
>> Fix this issue by enabling starfive pcie controller device's PM runtime
>> status before calling into pci_host_probe() through plda_pcie_host_init().
>>
>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Fixes: 02787a3b4d10 ("PCI/PM: Enable runtime power management for host bridges")
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>>   drivers/pci/controller/plda/pcie-starfive.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
>> index 0567ec373a3e..e73c1b7bc8ef 100644
>> --- a/drivers/pci/controller/plda/pcie-starfive.c
>> +++ b/drivers/pci/controller/plda/pcie-starfive.c
>> @@ -404,6 +404,9 @@ static int starfive_pcie_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	pm_runtime_enable(&pdev->dev);
>> +	pm_runtime_get_sync(&pdev->dev);
>> +
>>   	plda->host_ops = &sf_host_ops;
>>   	plda->num_events = PLDA_MAX_EVENT_NUM;
>>   	/* mask doorbell event */
>> @@ -413,11 +416,12 @@ static int starfive_pcie_probe(struct platform_device *pdev)
>>   	plda->events_bitmap <<= PLDA_NUM_DMA_EVENTS;
>>   	ret = plda_pcie_host_init(&pcie->plda, &starfive_pcie_ops,
>>   				  &stf_pcie_event);
>> -	if (ret)
>> +	if (ret) {
>> +		pm_runtime_put_sync(&pdev->dev);
>> +		pm_runtime_disable(&pdev->dev);
>>   		return ret;
>> +	}
>>   
>> -	pm_runtime_enable(&pdev->dev);
>> -	pm_runtime_get_sync(&pdev->dev);
>>   	platform_set_drvdata(pdev, pcie);
>>   
>>   	return 0;
>> -- 
>> 2.25.1
>>
Re: [PATCH] PCI: starfive: Enable PCIe controller's PM runtime before probing host bridge
Posted by Bjorn Helgaas 1 month, 2 weeks ago
On Fri, Oct 11, 2024 at 04:14:10PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 10/11/2024 2:22 AM, Bjorn Helgaas wrote:
> > On Thu, Oct 10, 2024 at 01:29:50PM -0700, Mayank Rana wrote:
> > > Commit 02787a3b4d10 ("PCI/PM: Enable runtime power management for host
> > > bridges") enables runtime PM for host bridge enforcing dependency chain
> > > between PCIe controller, host bridge and endpoint devices. With this,
> > > Starfive PCIe controller driver's probe enables host bridge (child device)
> > > PM runtime before parent's PM runtime (Starfive PCIe controller device)
> > > causing below warning and callstack:
> > 
> > I don't want the bisection hole that would result if we kept
> > 02787a3b4d10 ("PCI/PM: Enable runtime power management for host
> > bridges") and applied this patch on top of it.
> > 
> > If this is the fix, we'll apply it *first*, followed by 02787a3b4d10
> > (which will obviously become a different commit), so the locking
> > problem below described below should never exist in -next or the
> > upstream tree.
> > 
> > So we need to audit other drivers to make sure they don't have theBjorn, I have checked all the drivers in the controller folder where
> they are using pm_runtime_enable(), this is the only driver which needs
> to be fixed. once this patched was taken can we take "PCI/PM: Enable
>  runtime power management for host bridges"

Since these need to be applied in order, the usual process is to post
them together in one series.  Please work with Mayank to revise the
commit log of the starfive patch so it explains why the change is
necessary *independent* of your patch, and then post it and your
"enable runtime PM for host bridges" together as v6 of your patch.

In the cover letter include a note about why no other drivers need a
change like starfive does and how we can verify that.

Bjorn
Re: [PATCH] PCI: starfive: Enable PCIe controller's PM runtime before probing host bridge
Posted by Mayank Rana 1 month, 2 weeks ago
Hi Bjorn

On 10/10/2024 1:52 PM, Bjorn Helgaas wrote:
> On Thu, Oct 10, 2024 at 01:29:50PM -0700, Mayank Rana wrote:
>> Commit 02787a3b4d10 ("PCI/PM: Enable runtime power management for host
>> bridges") enables runtime PM for host bridge enforcing dependency chain
>> between PCIe controller, host bridge and endpoint devices. With this,
>> Starfive PCIe controller driver's probe enables host bridge (child device)
>> PM runtime before parent's PM runtime (Starfive PCIe controller device)
>> causing below warning and callstack:
> 
> I don't want the bisection hole that would result if we kept
> 02787a3b4d10 ("PCI/PM: Enable runtime power management for host
> bridges") and applied this patch on top of it.
> 
> If this is the fix, we'll apply it *first*, followed by 02787a3b4d10
> (which will obviously become a different commit), so the locking
> problem below described below should never exist in -next or the
> upstream tree.
> 
> So we need to audit other drivers to make sure they don't have the
> same problem as starfive, then make a series with this patch and any
> others that need similar fixes, followed by the "PCI/PM: Enable
> runtime power management for host bridges" patch.
> 
> Presumably this patch fixes a problem all by itself, even without
> considering 02787a3b4d10, so the commit message should describe that
> problem.

Ok. Thank you for quick response and above suggestion.
I shall reword this patch commit text, and resend it.

Regards,
Mayank
>> pcie-starfive 940000000.pcie: Enabling runtime PM for inactive device
>> with active children
>>
>> ======================================================
>> WARNING: possible circular locking dependency detected
>> 6.12.0-rc1+ #15438 Not tainted
>> ------------------------------------------------------
>> systemd-udevd/159 is trying to acquire lock:
>> ffffffff81822520 (console_owner){-.-.}-{0:0}, at:
>> console_lock_spinning_enable+0x3a/0x60
>>
>> but task is already holding lock:
>> ffffffd6c0b3d980 (&dev->power.lock){-...}-{2:2}, at:
>> pm_runtime_enable+0x1e/0xb6
>>
>> which lock already depends on the new lock.
>>
>> the existing dependency chain (in reverse order) is:
>>
>> -> #2 (&dev->power.lock){-...}-{2:2}:
>>          lock_acquire.part.0+0xa2/0x1d4
>>          lock_acquire+0x44/0x5a
>>          _raw_spin_lock_irqsave+0x3a/0x64
>>          __pm_runtime_resume+0x40/0x86
>>          __uart_start+0x40/0xb2
>>          uart_write+0x90/0x220
>>          n_tty_write+0x10a/0x40e
>>          file_tty_write.constprop.0+0x10c/0x230
>>          redirected_tty_write+0x84/0xbc
>>          do_iter_readv_writev+0x100/0x166
>>          vfs_writev+0xc6/0x398
>>          do_writev+0x5c/0xca
>>          __riscv_sys_writev+0x16/0x1e
>>          do_trap_ecall_u+0x1b6/0x1e2
>>          _new_vmalloc_restore_context_a0+0xc2/0xce
>>
>> -> #1 (&port_lock_key){-.-.}-{2:2}:
>>          lock_acquire.part.0+0xa2/0x1d4
>>          lock_acquire+0x44/0x5a
>>          _raw_spin_lock_irqsave+0x3a/0x64
>>          serial8250_console_write+0x2a0/0x474
>>          univ8250_console_write+0x22/0x2a
>>          console_flush_all+0x2f6/0x3c8
>>          console_unlock+0x80/0x1a8
>>          vprintk_emit+0x10e/0x2e0
>>          vprintk_default+0x16/0x1e
>>          vprintk+0x1e/0x3c
>>          _printk+0x36/0x50
>>          register_console+0x292/0x418
>>          serial_core_register_port+0x6d6/0x6dc
>>          serial_ctrl_register_port+0xc/0x14
>>          uart_add_one_port+0xc/0x14
>>          serial8250_register_8250_port+0x288/0x428
>>          dw8250_probe+0x422/0x518
>>          platform_probe+0x4e/0x92
>>          really_probe+0x10a/0x2da
>>          __driver_probe_device.part.0+0xb2/0xe8
>>          driver_probe_device+0x78/0xc4
>>          __device_attach_driver+0x66/0xc6
>>          bus_for_each_drv+0x5c/0xb0
>>          __device_attach+0x84/0x13c
>>          device_initial_probe+0xe/0x16
>>          bus_probe_device+0x88/0x8a
>>          deferred_probe_work_func+0xd4/0xee
>>          process_one_work+0x1e0/0x534
>>          worker_thread+0x166/0x2cc
>>          kthread+0xc4/0xe0
>>          ret_from_fork+0xe/0x18
>>
>> -> #0 (console_owner){-.-.}-{0:0}:
>>          check_noncircular+0x10e/0x122
>>          __lock_acquire+0x105c/0x1f4a
>>          lock_acquire.part.0+0xa2/0x1d4
>>          lock_acquire+0x44/0x5a
>>          console_lock_spinning_enable+0x58/0x60
>>          console_flush_all+0x2cc/0x3c8
>>          console_unlock+0x80/0x1a8
>>          vprintk_emit+0x10e/0x2e0
>>          dev_vprintk_emit+0xea/0x112
>>          dev_printk_emit+0x2e/0x48
>>          __dev_printk+0x40/0x5c
>>          _dev_warn+0x46/0x60
>>          pm_runtime_enable+0x98/0xb6
>>          starfive_pcie_probe+0x12e/0x228 [pcie_starfive]
>>          platform_probe+0x4e/0x92
>>          really_probe+0x10a/0x2da
>>          __driver_probe_device.part.0+0xb2/0xe8
>>          driver_probe_device+0x78/0xc4
>>          __driver_attach+0x54/0x162
>>          bus_for_each_dev+0x58/0xa4
>>          driver_attach+0x1a/0x22
>>          bus_add_driver+0xec/0x1ce
>>          driver_register+0x3e/0xd8
>>          __platform_driver_register+0x1c/0x24
>>          starfive_pcie_driver_init+0x20/0x1000 [pcie_starfive]
>>          do_one_initcall+0x5e/0x28c
>>          do_init_module+0x52/0x1ba
>>          load_module+0x1440/0x18f0
>>          init_module_from_file+0x76/0xae
>>          idempotent_init_module+0x18c/0x24a
>>          __riscv_sys_finit_module+0x52/0x82
>>          do_trap_ecall_u+0x1b6/0x1e2
>>          _new_vmalloc_restore_context_a0+0xc2/0xce
>>
>> other info that might help us debug this:
>>
>> Chain exists of:
>>     console_owner --> &port_lock_key --> &dev->power.lock
>>
>>    Possible unsafe locking scenario:
>>
>>          CPU0                    CPU1
>>          ----                    ----
>>     lock(&dev->power.lock);
>>                                  lock(&port_lock_key);
>>                                  lock(&dev->power.lock);
>>     lock(console_owner);
>>
>>    *** DEADLOCK ***
>>
>> 4 locks held by systemd-udevd/159:
>>    #0: ffffffd6c0b3d8f8 (&dev->mutex){....}-{3:3}, at:
>> __driver_attach+0x4c/0x162
>>    #1: ffffffd6c0b3d980 (&dev->power.lock){-...}-{2:2}, at:
>> pm_runtime_enable+0x1e/0xb6
>>    #2: ffffffff818223b0 (console_lock){+.+.}-{0:0}, at:
>> dev_vprintk_emit+0xea/0x112
>>    #3: ffffffff81822448 (console_srcu){....}-{0:0}, at:
>> console_flush_all+0x4e/0x3c8
>>
>> stack backtrace:
>> CPU: 1 UID: 0 PID: 159 Comm: systemd-udevd Not tainted 6.12.0-rc1+ #15438
>> Hardware name: StarFive VisionFive 2 v1.2A (DT)
>> Call Trace:
>> [<ffffffff80006a02>] dump_backtrace+0x1c/0x24
>> [<ffffffff80b70b3e>] show_stack+0x2c/0x38
>> [<ffffffff80b7f8f8>] dump_stack_lvl+0x7a/0xb4
>> [<ffffffff80b7f946>] dump_stack+0x14/0x1c
>> [<ffffffff8007fbc2>] print_circular_bug+0x2aa/0x350
>> [<ffffffff8007fd76>] check_noncircular+0x10e/0x122
>> [<ffffffff80082a3c>] __lock_acquire+0x105c/0x1f4a
>> [<ffffffff80084148>] lock_acquire.part.0+0xa2/0x1d4
>> [<ffffffff800842be>] lock_acquire+0x44/0x5a
>> [<ffffffff8008b3e8>] console_lock_spinning_enable+0x58/0x60
>> [<ffffffff8008c0c2>] console_flush_all+0x2cc/0x3c8
>> [<ffffffff8008c23e>] console_unlock+0x80/0x1a8
>> [<ffffffff8008c710>] vprintk_emit+0x10e/0x2e0
>> [<ffffffff80b79ec8>] dev_vprintk_emit+0xea/0x112
>> [<ffffffff80b79f1e>] dev_printk_emit+0x2e/0x48
>> [<ffffffff80b7a006>] __dev_printk+0x40/0x5c
>> [<ffffffff80b7a2be>] _dev_warn+0x46/0x60
>> [<ffffffff807037ae>] pm_runtime_enable+0x98/0xb6
>> [<ffffffff02763240>] starfive_pcie_probe+0x12e/0x228 [pcie_starfive]
>> [<ffffffff806f83f6>] platform_probe+0x4e/0x92
>> [<ffffffff80b7a680>] really_probe+0x10a/0x2da
>> [<ffffffff80b7a902>] __driver_probe_device.part.0+0xb2/0xe8
>> [<ffffffff806f6112>] driver_probe_device+0x78/0xc4
>> [<ffffffff806f6278>] __driver_attach+0x54/0x162
>> [<ffffffff806f42e6>] bus_for_each_dev+0x58/0xa4
>> [<ffffffff806f5c9e>] driver_attach+0x1a/0x22
>> [<ffffffff806f54ce>] bus_add_driver+0xec/0x1ce
>> [<ffffffff806f7112>] driver_register+0x3e/0xd8
>> [<ffffffff806f80cc>] __platform_driver_register+0x1c/0x24
>> [<ffffffff027ea020>] starfive_pcie_driver_init+0x20/0x1000 [pcie_starfive]
>> [<ffffffff800027ba>] do_one_initcall+0x5e/0x28c
>> [<ffffffff800bb4d8>] do_init_module+0x52/0x1ba
>> [<ffffffff800bcc8a>] load_module+0x1440/0x18f0
>> [<ffffffff800bd2f0>] init_module_from_file+0x76/0xae
>> [<ffffffff800bd4b4>] idempotent_init_module+0x18c/0x24a
>> [<ffffffff800bd5fc>] __riscv_sys_finit_module+0x52/0x82
>> [<ffffffff80b804a0>] do_trap_ecall_u+0x1b6/0x1e2
>> [<ffffffff80b8c536>] _new_vmalloc_restore_context_a0+0xc2/0xce
>>
>> Fix this issue by enabling starfive pcie controller device's PM runtime
>> status before calling into pci_host_probe() through plda_pcie_host_init().
>>
>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Fixes: 02787a3b4d10 ("PCI/PM: Enable runtime power management for host bridges")
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>>   drivers/pci/controller/plda/pcie-starfive.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
>> index 0567ec373a3e..e73c1b7bc8ef 100644
>> --- a/drivers/pci/controller/plda/pcie-starfive.c
>> +++ b/drivers/pci/controller/plda/pcie-starfive.c
>> @@ -404,6 +404,9 @@ static int starfive_pcie_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	pm_runtime_enable(&pdev->dev);
>> +	pm_runtime_get_sync(&pdev->dev);
>> +
>>   	plda->host_ops = &sf_host_ops;
>>   	plda->num_events = PLDA_MAX_EVENT_NUM;
>>   	/* mask doorbell event */
>> @@ -413,11 +416,12 @@ static int starfive_pcie_probe(struct platform_device *pdev)
>>   	plda->events_bitmap <<= PLDA_NUM_DMA_EVENTS;
>>   	ret = plda_pcie_host_init(&pcie->plda, &starfive_pcie_ops,
>>   				  &stf_pcie_event);
>> -	if (ret)
>> +	if (ret) {
>> +		pm_runtime_put_sync(&pdev->dev);
>> +		pm_runtime_disable(&pdev->dev);
>>   		return ret;
>> +	}
>>   
>> -	pm_runtime_enable(&pdev->dev);
>> -	pm_runtime_get_sync(&pdev->dev);
>>   	platform_set_drvdata(pdev, pcie);
>>   
>>   	return 0;
>> -- 
>> 2.25.1
>>