linux-kernel - Re: [PATCH v2] PCI: cadence: Enable support for applying lane equalization presets

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] [day] [month] [year] [list]

Message-ID: <9b9f4bb33620bbd30d4fc8d440f6e8fab71ed6ff.camel@ti.com>
Date: Thu, 6 Nov 2025 12:23:30 +0530
From: Siddharth Vadapalli <s-vadapalli@...com>
To: Manivannan Sadhasivam <mani@...nel.org>
CC: <lpieralisi@...nel.org>, <kwilczynski@...nel.org>, <robh@...nel.org>,
	<bhelgaas@...gle.com>, <kishon@...nel.org>, <18255117159@....com>,
	<unicorn_wang@...look.com>, <linux-pci@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
	<srk@...com>, <s-vadapalli@...com>
Subject: Re: [PATCH v2] PCI: cadence: Enable support for applying lane
 equalization presets

On Wed, 2025-11-05 at 20:40 +0530, Manivannan Sadhasivam wrote:
> On Tue, Oct 28, 2025 at 07:15:58PM +0530, Siddharth Vadapalli wrote:
> > The PCIe Link Equalization procedure allows peers on a PCIe Link to
> > improve the signal quality by exchanging transmitter presets and
> > receiver preset hints in the form of Ordered Sets.
> > 
> > For link speeds of 8.0 GT/s and above, the transmitter presets and the
> > receiver preset hints are configurable parameters which can be tuned to
> > establish a stable link. This allows setting up a stable link that is
> > specific to the peers across a Link.
> > 
> > The device-tree property 'eq-presets-Ngts' (eq-presets-8gts,
> > eq-presets-16gts, ...) specifies the transmitter presets and receiver
> > preset hints to be applied to every lane of the link for every supported
> > link speed that is greater than or equal to 8.0 GT/s.
> > 
> > Hence, enable support for applying the 'optional' lane equalization
> > presets when operating in the Root-Port (Root-Complex / Host) mode.
> > 
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@...com>
> > ---
> > 
> > Hello,
> > 
> > This patch is based on linux-next tagged next-20251028.
> 
> Just rebase on top of pci/main or any topic branches in pci tree if there are
> any dependency.

There are no dependencies. I tested on top of linux-next to see if there
may be any issues with changes from other subsystems. Also, I ensured that
the patch applies on v6.18-rc1 as stated below.

> 
> > It also applies cleanly on v6.18-rc1.
> > 
> > Link to v1 patch:
> > https://lore.kernel.org/r/20251027133013.2589119-1-s-vadapalli@ti.com/
> > Changes since v1:
> > - Implemented Bjorn's suggestion of adding 'fallthrough' keyword in
> >   switch-case to avoid compilation warnings, since 'fallthrough' is
> >   intentional.
> > 
> > Regards,
> > Siddharth.
> > 
> >  .../controller/cadence/pcie-cadence-host.c    | 85 +++++++++++++++++++
> >  drivers/pci/controller/cadence/pcie-cadence.h |  5 ++
> >  2 files changed, 90 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> > index fffd63d6665e..ae85ad8cce82 100644
> > --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> > +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> > @@ -168,6 +168,90 @@ static void cdns_pcie_host_enable_ptm_response(struct cdns_pcie *pcie)
> >  	cdns_pcie_writel(pcie, CDNS_PCIE_LM_PTM_CTRL, val | CDNS_PCIE_LM_TPM_CTRL_PTMRSEN);
> >  }
> >  
> > +static void cdns_pcie_setup_lane_equalization_presets(struct cdns_pcie_rc *rc)
> > +{
> > +	struct cdns_pcie *pcie = &rc->pcie;
> > +	struct device *dev = pcie->dev;
> > +	struct device_node *np = dev->of_node;
> > +	int max_link_speed, max_lanes, ret;
> > +	u32 lane_eq_ctrl_reg;
> > +	u16 cap;
> > +	u16 *presets_8gts;
> > +	u8 *presets_ngts;
> > +	u8 i, j;
> > +
> > +	ret = of_property_read_u32(np, "num-lanes", &max_lanes);
> > +	if (ret)
> > +		return;
> > +
> > +	/* Lane Equalization presets are optional, so error message is not necessary */
> > +	ret = of_pci_get_equalization_presets(dev, &rc->eq_presets, max_lanes);
> > +	if (ret)
> > +		return;
> > +
> > +	max_link_speed = of_pci_get_max_link_speed(np);
> 
> 'max-link-speed' property is used to *limit* the max link speed of the
> controller, in case the hardware default value is wrong or to workaround the
> hardware defect. If you goal is to find the actual max link speed of the Root
> Port, you should read the Link Capabilities register.

The intent was to get the maximum link speed that the user wants the
Controller to support. The idea is that the user will only specify the
presets in the device-tree ranging from 8.0 GT/s until the maximum link
speed that they desire. The Controller could support a higher link speed in
Hardware, but if the user doesn't intend to enable it, the presets won't be
present in the device-tree anyway.

> 
> Unfortunately, the ti,j721e-pci-host.yaml binding has marked this property as
> 'required'. It should've been optional instead.
> 
> > +	if (max_link_speed < 0) {
> > +		dev_err(dev, "%s: link-speed unknown, skipping preset setup\n", __func__);
> 
> Don't print the function names in error log.

I will fix this in v3. Thank you for reviewing the patch. Please let me
know if you have further feedback regarding my comment above on the
'maximum link speed'.

Regards,
Siddharth.