On Thu, Jun 26, 2025 at 10:50:31PM +0800, Hans Zhang wrote: > i.MX6 PCIe driver contains multiple read-modify-write sequences for > link training and speed configuration. These operations manually handle > bit masking and shifting to update specific fields in control registers, > particularly for link capabilities and speed change initiation. > > Refactor link capability configuration and speed change handling using > dw_pcie_clear_and_set_dword(). The helper simplifies LNKCAP modification > by encapsulating bit clear/set operations and eliminates intermediate > variables. For speed change control, replace explicit bit manipulation > with direct register updates through the helper. > > Adopting the standard interface reduces code complexity in link training > paths and ensures consistent handling of speed-related bits. The change > also prepares the driver for future enhancements to Gen3 link training > by centralizing bit manipulation logic. > > Signed-off-by: Hans Zhang <18255117159@xxxxxxx> missed my review tag, you need collect all review tags when respin patches. Reviewed-by: Frank Li <Frank.Li@xxxxxxx> https://lore.kernel.org/linux-pci/aFQp4MYpRaEUXNQy@lizhi-Precision-Tower-5810/ > --- > drivers/pci/controller/dwc/pci-imx6.c | 26 ++++++++++---------------- > 1 file changed, 10 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 5a38cfaf989b..3004e432f013 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -941,7 +941,6 @@ static int imx_pcie_start_link(struct dw_pcie *pci) > struct imx_pcie *imx_pcie = to_imx_pcie(pci); > struct device *dev = pci->dev; > u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); > - u32 tmp; > int ret; > > if (!(imx_pcie->drvdata->flags & > @@ -956,10 +955,9 @@ static int imx_pcie_start_link(struct dw_pcie *pci) > * bus will not be detected at all. This happens with PCIe switches. > */ > dw_pcie_dbi_ro_wr_en(pci); > - tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP); > - tmp &= ~PCI_EXP_LNKCAP_SLS; > - tmp |= PCI_EXP_LNKCAP_SLS_2_5GB; > - dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp); > + dw_pcie_clear_and_set_dword(pci, offset + PCI_EXP_LNKCAP, > + PCI_EXP_LNKCAP_SLS, > + PCI_EXP_LNKCAP_SLS_2_5GB); > dw_pcie_dbi_ro_wr_dis(pci); > > /* Start LTSSM. */ > @@ -972,18 +970,16 @@ static int imx_pcie_start_link(struct dw_pcie *pci) > > /* Allow faster modes after the link is up */ > dw_pcie_dbi_ro_wr_en(pci); > - tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP); > - tmp &= ~PCI_EXP_LNKCAP_SLS; > - tmp |= pci->max_link_speed; > - dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp); > + dw_pcie_clear_and_set_dword(pci, offset + PCI_EXP_LNKCAP, > + PCI_EXP_LNKCAP_SLS, > + pci->max_link_speed); > > /* > * Start Directed Speed Change so the best possible > * speed both link partners support can be negotiated. > */ > - tmp = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL); > - tmp |= PORT_LOGIC_SPEED_CHANGE; > - dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp); > + dw_pcie_clear_and_set_dword(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, > + 0, PORT_LOGIC_SPEED_CHANGE); > dw_pcie_dbi_ro_wr_dis(pci); > > ret = imx_pcie_wait_for_speed_change(imx_pcie); > @@ -1295,7 +1291,6 @@ static void imx_pcie_host_post_init(struct dw_pcie_rp *pp) > { > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > struct imx_pcie *imx_pcie = to_imx_pcie(pci); > - u32 val; > > if (imx_pcie->drvdata->flags & IMX_PCIE_FLAG_8GT_ECN_ERR051586) { > /* > @@ -1310,9 +1305,8 @@ static void imx_pcie_host_post_init(struct dw_pcie_rp *pp) > * to 0. > */ > dw_pcie_dbi_ro_wr_en(pci); > - val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF); > - val &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL; > - dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val); > + dw_pcie_clear_and_set_dword(pci, GEN3_RELATED_OFF, > + GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL, 0); > dw_pcie_dbi_ro_wr_dis(pci); > } > } > -- > 2.25.1 >
