[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e7cttuu5525k7ryvmkh6ifspm5vloj4muazvmp3yxtkgsdtbjs@u4odsfurejxq>
Date: Fri, 31 Oct 2025 20:23:27 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Anand Moon <linux.amoon@...il.com>
Cc: Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kwilczynski@...nel.org>, Rob Herring <robh@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Heiko Stuebner <heiko@...ech.de>,
Niklas Cassel <cassel@...nel.org>, Shawn Lin <shawn.lin@...k-chips.com>,
Hans Zhang <18255117159@....com>, Nicolas Frattaroli <nicolas.frattaroli@...labora.com>,
"open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS" <linux-pci@...r.kernel.org>,
"moderated list:ARM/Rockchip SoC support" <linux-arm-kernel@...ts.infradead.org>,
"open list:ARM/Rockchip SoC support" <linux-rockchip@...ts.infradead.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] PCI: dw-rockchip: Add runtime PM support to
Rockchip PCIe driver
On Fri, Oct 31, 2025 at 07:33:23PM +0530, Anand Moon wrote:
> Hi Manivannan
>
> Thanks for your review comment.
>
> On Fri, 31 Oct 2025 at 14:09, Manivannan Sadhasivam <mani@...nel.org> wrote:
> >
> > On Mon, Oct 27, 2025 at 08:25:30PM +0530, Anand Moon wrote:
> > > Add runtime power management support to the Rockchip DesignWare PCIe
> > > controller driver by enabling devm_pm_runtime() in the probe function.
> > > These changes allow the PCIe controller to suspend and resume dynamically,
> > > improving power efficiency on supported platforms.
> > >
> >
> > Seriously? How can this patch improve the power efficiency if it is not doing
> > any PM operation on its own?
> >
> I could verify that runtime power management is active
>
This is runtime status being active, which is a different thing as it only
allows the runtime PM hierarchy to be maintained. But the way you described the
commit message sounds like the patch is enabling runtime PM of the controller
and that improves efficiency (as if the controller driver is actively doing
runtime PM operations).
> [root@...kpi-5b alarm]# cat
> /sys/devices/platform/a41000000.pcie/power/runtime_status
> active
> [root@...kpi-5b alarm]# find /sys -name runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/pci_bus/0004:41/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-3::lan/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-2::lan/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-1::lan/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-0::lan/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/r8169-4-4100:00/power/runtime_status
> /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/r8169-4-4100:00/hwmon/hwmon11/power/runtime_status
>
> Well, the powertop shows that the runtime power management is enabled
> on Radxa Rock 5b,
>
> PowerTOP 2.15 Overview Idle stats Frequency stats Device
> stats Device Freq stats Tunables WakeUp
> >> Good Wireless Power Saving for interface wlan0
> Good VM writeback timeout
> Good Bluetooth device interface status
> Good NMI watchdog should be turned off
> Good Autosuspend for unknown USB device 2-1.3 (8087:0a2b)
> Good Autosuspend for USB device USB 2.0 Hub [2-1]
> Good Autosuspend for USB device Generic Platform OHCI
> controller [usb1]
> Good Autosuspend for USB device xHCI Host Controller [usb8]
> Good Autosuspend for USB device Generic Platform OHCI
> controller [usb4]
> Good Autosuspend for USB device EHCI Host Controller [usb2]
> Good Autosuspend for USB device xHCI Host Controller [usb6]
> Good Autosuspend for USB device EHCI Host Controller [usb3]
> Good Autosuspend for USB device xHCI Host Controller [usb5]
> Good Autosuspend for USB device xHCI Host Controller [usb7]
> Good Runtime PM for PCI Device Intel Corporation Wireless
> 8265 / 8275
> Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588
> Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588
> Good Runtime PM for PCI Device Realtek Semiconductor Co.,
> Ltd. RTL8125 2.5GbE Controller
> Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588
> Good Runtime PM for PCI Device Samsung Electronics Co Ltd
> NVMe SSD Controller SM981/PM981/PM983
>
> PowerTOP 2.15 Overview Idle stats Frequency stats Device
> stats Device Freq stats Tunables WakeUp
> Usage Device name
> 1.1% CPU use
> 100.0% Radio device: rfkill_gpio
> 100.0% runtime-rockchip-gate-link-clk.712
> 100.0% PCI Device: Realtek Semiconductor Co., Ltd.
> RTL8125 2.5GbE Controller
> 100.0% runtime-rockchip-gate-link-clk.717
> 100.0% runtime-rockchip-gate-link-clk.714
> 100.0% runtime-rockchip-gate-link-clk.489
> 100.0% runtime-a40000000.pcie
> 100.0% runtime-a40800000.pcie
> 100.0% runtime-rockchip-gate-link-clk.718
> 100.0% runtime-rockchip-gate-link-clk.706
> 100.0% runtime-rockchip-gate-link-clk.708
> 100.0% PCI Device: Intel Corporation Wireless 8265 / 8275
> 100.0% Radio device: btusb
> 100.0% runtime-fcd00000.usb
> 100.0% PCI Device: Samsung Electronics Co Ltd NVMe
> SSD Controller SM981/PM981/PM983
> 100.0% Radio device: rfkill_gpio
> 100.0% runtime-fc000000.usb
> 100.0% Radio device: iwlwifi
> 100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588
> 100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588
> 100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588
> 100.0% runtime-rockchip-gate-link-clk.711
> 100.0% runtime-fc400000.usb
> 100.0% runtime-rockchip-gate-link-clk.704
> 100.0% runtime-rockchip-gate-link-clk.701
> 100.0% runtime-rockchip-gate-link-clk.716
> 100.0% runtime-rockchip-gate-link-clk.707
> 100.0% runtime-rockchip-gate-link-clk.709
> 100.0% runtime-rockchip-gate-link-clk.719
> 100.0% runtime-xhci-hcd.1.auto
> 100.0% runtime-feb50000.serial
> 100.0% runtime-rockchip-gate-link-clk.715
> 100.0% runtime-rockchip-gate-link-clk.710
>
> > Again, a pointless patch.
This patch might make sense on its own, to enable runtime PM status of the
controller so that the runtime PM could be applied to the entire PCIe hierarchy.
> I implemented a .remove patch to ensure proper resource cleanup,
> which is a necessary step for successfully enabling and managing
> runtime power for the device.
How a dead code (remove callback for a always built-in driver) becomes necessary
for runtime PM.
- Mani
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists