Re: [PATCH v8 4/4] PCI: dwc: Add support for configuring lane equalization presets
From: Manivannan Sadhasivam
Date: Fri Mar 28 2025 - 02:45:52 EST
On Fri, Mar 28, 2025 at 11:04:11AM +0530, Krishna Chaitanya Chundru wrote:
>
>
> On 3/28/2025 10:23 AM, Manivannan Sadhasivam wrote:
> > On Sun, Mar 16, 2025 at 09:39:04AM +0530, Krishna Chaitanya Chundru wrote:
> > > PCIe equalization presets are predefined settings used to optimize
> > > signal integrity by compensating for signal loss and distortion in
> > > high-speed data transmission.
> > >
> > > Based upon the number of lanes and the data rate supported, write
> > > the preset data read from the device tree in to the lane equalization
> > > control registers.
> > >
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware-host.c | 60 +++++++++++++++++++++++
> > > drivers/pci/controller/dwc/pcie-designware.h | 3 ++
> > > include/uapi/linux/pci_regs.h | 3 ++
> > > 3 files changed, 66 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index dd56cc02f4ef..7c6e6a74383b 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -507,6 +507,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> > > if (pci->num_lanes < 1)
> > > pci->num_lanes = dw_pcie_link_get_max_link_width(pci);
> > > + ret = of_pci_get_equalization_presets(dev, &pp->presets, pci->num_lanes);
> > > + if (ret)
> > > + goto err_free_msi;
> > > +
> > > /*
> > > * Allocate the resource for MSG TLP before programming the iATU
> > > * outbound window in dw_pcie_setup_rc(). Since the allocation depends
> > > @@ -808,6 +812,61 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> > > return 0;
> > > }
> > > +static void dw_pcie_program_presets(struct dw_pcie_rp *pp, enum pci_bus_speed speed)
> > > +{
> > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > + u8 lane_eq_offset, lane_reg_size, cap_id;
> > > + u8 *presets;
> > > + u32 cap;
> > > + int i;
> > > +
> > > + if (speed == PCIE_SPEED_8_0GT) {
> > > + presets = (u8 *)pp->presets.eq_presets_8gts;
> > > + lane_eq_offset = PCI_SECPCI_LE_CTRL;
> > > + cap_id = PCI_EXT_CAP_ID_SECPCI;
> > > + /* For data rate of 8 GT/S each lane equalization control is 16bits wide*/
> > > + lane_reg_size = 0x2;
> > > + } else if (speed == PCIE_SPEED_16_0GT) {
> > > + presets = pp->presets.eq_presets_Ngts[EQ_PRESET_TYPE_16GTS - 1];
> > > + lane_eq_offset = PCI_PL_16GT_LE_CTRL;
> > > + cap_id = PCI_EXT_CAP_ID_PL_16GT;
> > > + lane_reg_size = 0x1;
> > > + } else {
> >
> > Can you add conditions for other data rates also? Like 32, 64 GT/s. If
> > controller supports them and if the presets property is defined in DT, then you
> > should apply the preset values.
> >
> > If the presets property is not present in DT, then below 'PCI_EQ_RESV' will
> > safely return.
> >
> I am fine to add it, but there is no GEN5 or GEN6 controller support
> added in dwc, isn't it best to add when that support is added and
> tested.
>
What is the guarantee that this part of the code will be updated once the
capable controllers start showing up? I don't think there will be any issue in
writing to these registers.
- Mani
--
மணிவண்ணன் சதாசிவம்