On Thu, Sep 04, 2025 at 11:33:00AM GMT, Siddharth Vadapalli wrote:
On Wed, Sep 03, 2025 at 06:51:14PM +0530, Manivannan Sadhasivam wrote:
[...]
I thought put_noidle() will just reduce the refcount and not invoke the idle/suspend callbacks, but I seem to be wrong here.
Anyway, I guess we have no option here other than to drop the pm_runtime_put_noidle() call. This will keep refcount as 1 and will prevent the parent (genpd) to not enter runtime suspend, but we have to live with it (this was also the previous beahvior as well).
Btw, pm_runtime_set_active/enable change belongs to a separate patch.
I will work on and post the v4 patch with the following change:
- The commit message and the comment in the code will be updated to indicate that the runtime PM APIs are used to cause 'genpd' to power off and power on the controller.
I assume that the driver changes can remain as they are in this patch given that the existing behavior has to be preserved. As j721e_pcie_ctrl_init() programs the strap settings, powering off the controller via pm_runtime_put_sync() and powering it on via pm_runtime_get_sync() shall be implemented within j721e_pcie_ctrl_init() itself leaving the rest of the probe function as-is. To summarize, the driver changes will be:
j721e_pcie_ctrl_init() ...Existing code...
- Add pm_runtime_put_sync() here to power off controller ...Existing code to program strap settings...
- Add pm_runtime_get_sync() here to power on controller ...Existing code...
Please let me know what you think.
I would've preferred changing the runtime PM calls as I suggested initially, but still we would keep it incremented which brings the net effect to the same.
So you can drop my initial comment and post just this patch with improved patch description.
- Mani