On Wed, Jun 11, 2025 at 02:38:14PM -0400, Frank Li wrote: > On Thu, Jun 12, 2025 at 12:31:21AM +0800, Hans Zhang wrote: > > Subject should be > > PCI: dwc: imx6: Refactor code by using dw_pcie_clear_and_set_dword() > > tag "imx6:" should after "dwc:" > > Reviewed-by: Frank Li <Frank.Li@xxxxxxx> I sent out too quick, please don't applied my review tag at next version because I found a issue, see: https://lore.kernel.org/linux-pci/aEnONwJUSEMdMAUD@lizhi-Precision-Tower-5810/ Frank > > > 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> > > --- > > 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 > >
