On Wed, Jun 18, 2025 at 11:21:12PM +0800, Hans Zhang wrote:
> Tegra194 PCIe driver contains extensive manual bit manipulation across
> interrupt handling, ASPM configuration, and controller initialization.
> The driver implements complex read-modify-write sequences with explicit
> bit masking, leading to verbose and hard-to-maintain code.
>
> Refactor interrupt handling, ASPM setup, capability configuration, and
> controller initialization using dw_pcie_clear_and_set_dword(). Replace
> multi-step register modifications with single helper calls, eliminating
> intermediate variables and reducing code size by ~100 lines. For CDMA
> error handling, initialize the value variable to zero before setting
> status bits.
>
> This comprehensive refactoring significantly improves code readability
> and maintainability. Standardizing on the helper ensures consistent
> register access patterns across all driver components and reduces the
> risk of bit manipulation errors in this complex controller driver.
>
> Signed-off-by: Hans Zhang <18255117159@xxxxxxx>
> ---
> drivers/pci/controller/dwc/pcie-tegra194.c | 155 +++++++++------------
> 1 file changed, 64 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 4f26086f25da..c6f5c35a4be4 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -378,9 +378,8 @@ static irqreturn_t tegra_pcie_rp_irq_handler(int irq, void *arg)
> val |= APPL_CAR_RESET_OVRD_CYA_OVERRIDE_CORE_RST_N;
> appl_writel(pcie, val, APPL_CAR_RESET_OVRD);
>
> - val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> - val |= PORT_LOGIC_SPEED_CHANGE;
> - dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> + dw_pcie_clear_and_set_dword(pci, PCIE_LINK_WIDTH_SPEED_CONTROL,
> + 0, PORT_LOGIC_SPEED_CHANGE);
> }
> }
>
> @@ -412,7 +411,7 @@ static irqreturn_t tegra_pcie_rp_irq_handler(int irq, void *arg)
>
> if (status_l0 & APPL_INTR_STATUS_L0_CDM_REG_CHK_INT) {
> status_l1 = appl_readl(pcie, APPL_INTR_STATUS_L1_18);
> - val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS);
> + val = 0;
> if (status_l1 & APPL_INTR_STATUS_L1_18_CDM_REG_CHK_CMPLT) {
> dev_info(pci->dev, "CDM check complete\n");
> val |= PCIE_PL_CHK_REG_CHK_REG_COMPLETE;
> @@ -425,7 +424,8 @@ static irqreturn_t tegra_pcie_rp_irq_handler(int irq, void *arg)
> dev_err(pci->dev, "CDM Logic error\n");
> val |= PCIE_PL_CHK_REG_CHK_REG_LOGIC_ERROR;
> }
> - dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, val);
> + dw_pcie_clear_and_set_dword(pci, PCIE_PL_CHK_REG_CONTROL_STATUS,
> + PORT_LOGIC_SPEED_CHANGE, val);
I don't know why you are clearing PORT_LOGIC_SPEED_CHANGE here which is not part
of PCIE_PL_CHK_REG_CONTROL_STATUS. Typo?
- Mani
--
மணிவண்ணன் சதாசிவம்
