linux-kernel - Re: [PATCH v3] PCI: j721e: Fix programming sequence of "strap" settings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <08b87b5f-838b-4f40-ae90-10ded134356e@ti.com>
Date: Tue, 2 Sep 2025 10:34:51 +0530
From: Siddharth Vadapalli <s-vadapalli@...com>
To: Manivannan Sadhasivam <mani@...nel.org>
CC: Siddharth Vadapalli <s-vadapalli@...com>, <lpieralisi@...nel.org>,
        <kwilczynski@...nel.org>, <robh@...nel.org>, <bhelgaas@...gle.com>,
        <helgaas@...nel.org>, <kishon@...nel.org>, <vigneshr@...com>,
        <stable@...r.kernel.org>, <linux-pci@...r.kernel.org>,
        <linux-omap@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <srk@...com>
Subject: Re: [PATCH v3] PCI: j721e: Fix programming sequence of "strap"
 settings

On Mon, Sep 01, 2025 at 08:15:24PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Sep 01, 2025 at 04:50:02PM GMT, Siddharth Vadapalli wrote:
> > On Mon, Sep 01, 2025 at 12:14:51PM +0530, Manivannan Sadhasivam wrote:
> > > On Mon, Sep 01, 2025 at 11:51:33AM GMT, Siddharth Vadapalli wrote:

[...]

> > > > 
> > > > If I understand correctly, are you suggesting the following?
> > > > 
> > > > j721e_pcie_probe()
> > > > 	pm_runtime_set_active()
> > > > 	pm_runtime_enable()
> > > > 	ret = j721e_pcie_ctrl_init(pcie);
> > > > 		/*
> > > > 		 * PCIe Controller should be powered off here, but is there
> > > > 		 * a way to ensure that it has been powered off?
> > > > 		 */
> > > > 		=> Program the strap settings and return to
> > > > 		j721e_pcie_probe()
> > > > 	/* Power on the PCIe Controller now */
> > > > 	ret = pm_runtime_get_sync(dev);
> > > 
> > > This pm_runtime_get_sync() should be part of j721e_pcie_ctrl_init() where you
> > > do power off, program strap and power on.
> > > 
> > > This should not be part of probe() as by that time, controller is already
> > > powered on. So pm_runtime_set_active() and pm_runtime_enable() should be enough
> > > to convey the state of the device to PM core.
> > 
> > I have tried out the suggestion in the following manner:
> > 
> > 	j721e_pcie_probe()
> > 		...
> > 		pm_runtime_set_active(dev);
> > 		pm_runtime_enable(dev);
> > 		ret = j721e_pcie_ctrl_init(pcie);
> > 			... within j721e_pcie_ctrl_init()
> > 			ret = pm_runtime_put_sync(dev);
> > 			/* Program Strap Settings */
> > 			ret = pm_runtime_get_sync(dev);
> > 			...
> > 		...
> > 		exit probe
> > 
> > Since a 'pm_runtime_get_sync()' hasn't yet been invoked prior to the
> > section where 'pm_runtime_put_sync()' is invoked, it leads to a ref-count
> > underflow error at runtime. Please let me know if I am missing
> > something.
> > 
> 
> Doh... At the start of probe(), device PM usage_count will be 0. So we cannot
> decrement it without incrementing it.
> 
> Could you try below sequence?
> 
> 	probe()
> 	...
> 	pm_runtime_set_active()
> 	pm_runtime_enable()
> 	j721e_pcie_ctrl_init()
> 		...
> 		pm_runtime_get() /* Just increment usage_count */
> 		pm_runtime_put_sync() /* ask PM core to turn off */
> 		/* program strap setting */
> 		pm_runtime_get_sync() /* ask PM core to turn on */
> 		pm_runtime_put_noidle() /* balance the usage_count without
> 						suspending the device */
> 	...

The above sequence powers off the controller at the point in time that
the strap settings are programmed. 'pm_runtime_get_sync()' is powering
on the controller afterwards. However, the 'pm_runtime_put_noidle()'
at the end is causing the controller to be powered off again leading to
a crash. Removing 'pm_runtime_put_noidle()' results in a functional
sequence.

Please consider the existing sequence prior to this patch:

probe()
	...
	pm_runtime_enable()
	pm_runtime_get_sync() => usage_count is 1
	...
	exit probe

With the suggested sequence above, we have:

probe()
	...
	pm_runtime_set_active()
	pm_runtime_enable()
	j721e_pcie_ctrl_init()
		...
		pm_runtime_get() => usage_count is 1
		pm_runtime_put_sync() => usage_count is 0
		/* Controller is powered off now */
		/* Strap settings are programmed */
		pm_runtime_get_sync() => usage_count is 1
		/* Controller is powered on now */
		pm_runtime_put_noidle() => usage_count is 0
		/* Controller is powered off in a while */
	...
	/* Crash is observed aroung this point before probe finishes */

Please let me know if the fix is to drop 'pm_runtime_put_noidle()'
from the above sequence.

Regards,
Siddharth.