On Fri, Aug 29, 2025 at 02:46:28PM GMT, Siddharth Vadapalli wrote:
> The Cadence PCIe Controller integrated in the TI K3 SoCs supports both
> Root-Complex and Endpoint modes of operation. The Glue Layer allows
> "strapping" the Mode of operation of the Controller, the Link Speed
> and the Link Width. This is enabled by programming the "PCIEn_CTRL"
> register (n corresponds to the PCIe instance) within the CTRL_MMR
> memory-mapped register space. The "reset-values" of the registers are
> also different depending on the mode of operation.
>
> Since the PCIe Controller latches onto the "reset-values" immediately
> after being powered on, if the Glue Layer configuration is not done while
> the PCIe Controller is off, it will result in the PCIe Controller latching
> onto the wrong "reset-values". In practice, this will show up as a wrong
> representation of the PCIe Controller's capability structures in the PCIe
> Configuration Space. Some such capabilities which are supported by the PCIe
> Controller in the Root-Complex mode but are incorrectly latched onto as
> being unsupported are:
> - Link Bandwidth Notification
> - Alternate Routing ID (ARI) Forwarding Support
> - Next capability offset within Advanced Error Reporting (AER) capability
>
> Fix this by powering off the PCIe Controller before programming the "strap"
> settings and powering it on after that.
>
> Fixes: f3e25911a430 ("PCI: j721e: Add TI J721E PCIe driver")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@xxxxxx>
> ---
>
> Hello,
>
> This patch is based on commit
> 07d9df80082b Merge tag 'perf-tools-fixes-for-v6.17-2025-08-27' of git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools
> of Mainline Linux.
>
> v2 of this patch is at:
> https://lore.kernel.org/r/20250819101336.292013-1-s-vadapalli@xxxxxx/
> Changes since v2:
> - Based on Bjorn's feedback at:
> https://lore.kernel.org/r/20250819221748.GA598958@bhelgaas/
> 1) Commit message has been rephrased to summarize the issue and the
> fix without elaborating too much on the details.
> 2) Description of the issue's symptoms noticeable by a user has been
> added to the commit message.
> 3) Comment has been wrapped to fit within 80 columns.
> 4) The implementation has been simplified by moving the Controller
> Power OFF and Power ON sequence into j721e_pcie_ctrl_init() as a
> result of which the code reordering as well as function parameter
> changes are no longer required.
> - Based on offline feedback from Vignesh, Runtime PM APIs are used
> instead of PM DOMAIN APIs to power off and power on the PCIe
> Controller.
> - Rebased patch on latest Mainline Linux.
>
> Test Logs on J7200 EVM without the current patch applied show that the
> ARI Forwarding Capability incorrectly shows up as not being supported:
> https://gist.github.com/Siddharth-Vadapalli-at-TI/768bca36025ed630c4e69bcc3d94501a
>
> Test Logs on J7200 EVM with the current patch applied show that the
> ARI Forwarding Capability correctly shows up as being supported:
> https://gist.github.com/Siddharth-Vadapalli-at-TI/fc1752d17140646c8fa57209eccd86ce
>
> As explained in the commit message, this discrepancy is solely due to
> the PCIe Controller latching onto the incorrect reset-values which
> occurs when the strap settings are programmed after the PCIe Controller
> is powered on, at which point, the reset-values don't toggle anymore.
>
> Regards,
> Siddharth.
>
> drivers/pci/controller/cadence/pci-j721e.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> index 6c93f39d0288..c178b117215a 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -284,6 +284,22 @@ static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie)
> if (!ret)
> offset = args.args[0];
>
> + /*
> + * The PCIe Controller's registers have different "reset-values"
> + * depending on the "strap" settings programmed into the PCIEn_CTRL
> + * register within the CTRL_MMR memory-mapped register space.
> + * The registers latch onto a "reset-value" based on the "strap"
> + * settings sampled after the PCIe Controller is powered on.
> + * To ensure that the "reset-values" are sampled accurately, power
> + * off the PCIe Controller before programming the "strap" settings
> + * and power it on after that.
> + */
> + ret = pm_runtime_put_sync(dev);
> + if (ret < 0) {
> + dev_err(dev, "Failed to power off PCIe Controller\n");
> + return ret;
> + }
How does the controller gets powered off after pm_runtime_put_sync() since you
do not have runtime PM callbacks? I believe the parent is turning off the power
domain?
- Mani
--
மணிவண்ணன் சதாசிவம்
