linux-kernel - Re: [PATCH v2] PCI: cadence: Fix Gen2 Link Retraining process

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 for Android: free password hash cracker in your pocket

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

Message-ID: <646f0efe-d2b6-8bda-9629-fb9615cf4b52@ti.com>
Date:   Thu, 30 Mar 2023 09:52:06 +0530
From:   Siddharth Vadapalli <s-vadapalli@...com>
To:     Bjorn Helgaas <helgaas@...nel.org>
CC:     <tjoseph@...ence.com>, <lpieralisi@...nel.org>, <robh@...nel.org>,
        <kw@...ux.com>, <bhelgaas@...gle.com>, <nadeem@...ence.com>,
        "Raghavendra, Vignesh" <vigneshr@...com>,
        <linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <srk@...com>, <nm@...com>,
        <s-vadapalli@...com>
Subject: Re: [PATCH v2] PCI: cadence: Fix Gen2 Link Retraining process

Hello Bjorn,

On 29/03/23 22:38, Bjorn Helgaas wrote:
> On Wed, Mar 29, 2023 at 08:11:25PM +0530, Raghavendra, Vignesh wrote:
>> Hi Lorenzo, Bjorn,
>>
>> On 3/15/2023 12:38 PM, Siddharth Vadapalli wrote:
>>> The Link Retraining process is initiated to account for the Gen2 defect in
>>> the Cadence PCIe controller in J721E SoC. The errata corresponding to this
>>> is i2085, documented at:
>>> https://www.ti.com/lit/er/sprz455c/sprz455c.pdf
>>>
>>> The existing workaround implemented for the errata waits for the Data Link
>>> initialization to complete and assumes that the link retraining process
>>> at the Physical Layer has completed. However, it is possible that the
>>> Physical Layer training might be ongoing as indicated by the
>>> PCI_EXP_LNKSTA_LT bit in the PCI_EXP_LNKSTA register.
>>>
>>> Fix the existing workaround, to ensure that the Physical Layer training
>>> has also completed, in addition to the Data Link initialization.
>>>
>>> Fixes: 4740b969aaf5 ("PCI: cadence: Retrain Link to work around Gen2 training defect")
>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@...com>
>>> Reviewed-by: Vignesh Raghavendra <vigneshr@...com>
>>> ---
>>> Changes from v1:
>>> 1. Collect Reviewed-by tag from Vignesh Raghavendra.
>>> 2. Rebase on next-20230315.
>>>
>>> v1:
>>> https://lore.kernel.org/r/20230102075656.260333-1-s-vadapalli@ti.com
>>>
>>>  .../controller/cadence/pcie-cadence-host.c    | 27 +++++++++++++++++++
>>>  1 file changed, 27 insertions(+)
>>
>> Wondering do one of you be pulling this patch in? This patch was never
>> picked for 6.3-rc1 merge cycle... Just want to make sure
>> pcie-cadence*.c and pci-j721e.c patches have a path to reach pci tree.
> 
> Yes, Lorenzo or Krzysztof will likely pick this up.  I think Lorenzo
> is out of the office this week.
> 
> Drive-by comment: the current patch doesn't seem to give any
> indication to the user when cdns_pcie_host_training_complete() times
> out.  Is that timeout potentially of interest to a user?  Should there
> be a log message there?

Thank you for reviewing the patch. The return value of -ETIMEDOUT from the
function cdns_pcie_host_training_complete() added by this patch will be handled
similar to the -ETIMEDOUT from the cdns_pcie_host_wait_for_link() function that
is already present.

If cdns_pcie_host_training_complete() returns -ETIMEDOUT, it is returned to
cdns_pcie_host_start_link() function which is called within
cdns_pcie_host_setup() function. In the cdns_pcie_host_setup() function, there
is already a dev_dbg() print for handling the case where
cdns_pcie_host_wait_for_link() times out. For this reason, I felt that for both
cases, the dev_dbg() print can be used to debug without the need for an extra
log message. Please let me know if that's fine.

Regards,
Siddharth.