Re: [PATCH 7/8] PCI: xilinx-nwl: Reset the core during probe

Manivannan Sadhasivam
2026-02-18T16:43:08Z

On Mon, Feb 02, 2026 at 07:21:27PM -0500, Sean Anderson wrote:
> The PCIe core must be held in reset when initializing phys.
> Assert/deassert the appropriate resets.
>
> Resetting the core also resets the PCIe attributes to their default
> values, so initialize those too. For the most part the defaults are
> fine, but there are many attributes that default to an endpoint
> configuration and must be reprogrammed to function as a root device.
> We generally follow the controller programming sequence from UG1085.
>
> Xilinx was extremely imaginative and named all the registers ATTR_1,
> ATTR_2 etc. (with the fields organized in alphabetical order) so we
> follow the same convention. Only the fields are named, but sometimes a
> field is split across multiple registers. All the BARs are unused but
> some are repurposed as bridge registers when used as a root port.
>

Can someone from AMD/Xilinx review this patch?

- Mani

> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
>
> drivers/pci/controller/pcie-xilinx-nwl.c | 177 +++++++++++++++++++++++
> 1 file changed, 177 insertions(+)
>
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index 7cfdc21e6f40..b78fbad1efa5 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -22,6 +22,7 @@
> #include <linux/pci-ecam.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> +#include <linux/reset.h>
> #include <linux/irqchip/chained_irq.h>
>
> #include "../pci.h"
> @@ -133,6 +134,54 @@
> #define CFG_DMA_REG_BAR GENMASK(2, 0)
> #define CFG_PCIE_CACHE GENMASK(7, 0)
>
> +#define PCIE_ATTR2_AER_CAP_PERMIT_ROOTERR_UPDATE BIT(0)
> +
> +#define PCIE_ATTR25_CPL_TIMEOUT_DISABLE_SUPPORTED BIT(9)
> +#define PCIE_ATTR25_INTX_IMPLEMENTED BIT(8)
> +#define PCIE_ATTR25_CLASS_CODE GENMASK(7, 0)
> +
> +#define PCIE_ATTR27_DEV_CAP_ENDPOINT_L1_LATENCY GENMASK(5, 3)
> +
> +#define PCIE_ATTR34_HEADER_TYPE GENMASK(7, 0)
> +
> +#define PCIE_ATTR35_LINK_CAP_DLL_ACTIVE_REPORTING BIT(15)
> +
> +#define PCIE_ATTR37_LINK_CAP_MAX_LINK_SPEED GENMASK(13, 10)
> +#define PCIE_ATTR37_LINK_CAP_MAX_LINK_SPEED_2_5 1
> +#define PCIE_ATTR37_LINK_CAP_MAX_LINK_SPEED_5_0 2
> +#define PCIE_ATTR37_LINK_CAP_BANDWIDTH_NOTIFICATION BIT(9)
> +
> +#define PCIE_ATTR50_CAP_DEVICE_PORT_TYPE GENMASK(7, 4)
> +#define PCIE_ATTR50_CAP_NEXTPTR GENMASK(15, 8)
> +
> +#define PCIE_ATTR53_CAP_NEXTPTR GENMASK(7, 0)
> +
> +#define PCIE_ATTR93_LL_REPLAY_TIMEOUT_EN BIT(15)
> +
> +#define PCIE_ATTR97_LTSSM_MAX_LINK_WIDTH GENMASK(11, 6)
> +#define PCIE_ATTR97_LINK_CAP_MAX_LINK_WIDTH GENMASK(5, 0)
> +
> +#define PCIE_ATTR100_UPSTREAM_FACING BIT(6)
> +
> +#define PCIE_ATTR101_EN_MSG_ROUTE GENMASK(15, 5)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_PME_TURN_OFF BIT(15)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_UNLOCK BIT(14)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_PME_TO_ACK BIT(13)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_PM_PME BIT(12)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_INTD BIT(11)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_INTC BIT(10)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_INTB BIT(9)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_INTA BIT(8)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_ERR_FATAL BIT(7)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_ERR_NONFATAL BIT(6)
> +#define PCIE_ATTR101_EN_MSG_ROUTE_ERR_COR BIT(5)
> +#define PCIE_ATTR101_DISABLE_BAR_FILTERING BIT(1)
> +
> +#define PCIE_ATTR106_VC0_TOTAL_CREDITS_NPH GENMASK(13, 7)
> +#define PCIE_ATTR106_VC0_TOTAL_CREDITS_CH GENMASK(6, 0)
> +
> +#define PCIE_ATTR109_VC0_TOTAL_CREDITS_PH GENMASK(6, 0)
> +
> #define INT_PCI_MSI_NR (2 * 32)
>
> /* Readin the PS_LINKUP */
> @@ -159,6 +208,7 @@ struct nwl_pcie {
> void __iomem *pcireg_base;
> void __iomem *ecam_base;
> struct phy *phy[4];
> + struct reset_control *ctrl_reset;
> phys_addr_t phys_breg_base; /* Physical Bridge Register Base */
> phys_addr_t phys_pcie_reg_base; /* Physical PCIe Controller Base */
> phys_addr_t phys_ecam_base; /* Physical Configuration Base */
> @@ -173,6 +223,115 @@ struct nwl_pcie {
> raw_spinlock_t leg_mask_lock;
> };
>
> +static void nwl_pcie_write_attr(struct nwl_pcie *pcie, u32 attr, u16 val)
> +{
> + writel(val, pcie->pcireg_base + attr * 4);
> +}
> +
> +static void nwl_pcie_modify_attr(struct nwl_pcie *pcie, u32 attr, u16 clear,
> + u16 set)
> +{
> + u32 val = readl(pcie->pcireg_base + attr * 4);
> +
> + nwl_pcie_write_attr(pcie, attr, (val & ~clear) | set);
> +}
> +
> +static void nwl_pcie_attr_init(struct nwl_pcie *pcie)
> +{
> + unsigned int width;
> +
> + for (width = ARRAY_SIZE(pcie->phy); width; width--)
> + if (pcie->phy[width - 1])
> + break;
> +
> + /* Set TLP header to type-1 */
> + nwl_pcie_modify_attr(pcie, 34, PCIE_ATTR34_HEADER_TYPE, PCI_HEADER_TYPE_BRIDGE);
> + nwl_pcie_modify_attr(pcie, 100, PCIE_ATTR100_UPSTREAM_FACING, 0);
> +
> + /* Disable BAR0/1 */
> + nwl_pcie_write_attr(pcie, 7, 0);
> + nwl_pcie_write_attr(pcie, 8, 0);
> + nwl_pcie_write_attr(pcie, 9, 0);
> + nwl_pcie_write_attr(pcie, 10, 0);
> + /* Enable primary/secondary/subordinate busses, disable latency timer */
> + nwl_pcie_write_attr(pcie, 11, 0xffff);
> + nwl_pcie_write_attr(pcie, 12, 0x00ff);
> + nwl_pcie_write_attr(pcie, 13, 0x0000); /* Disable I/O window */
> + nwl_pcie_write_attr(pcie, 14, 0x0000); /* Enable secondary status */
> + /* Enable memory window */
> + nwl_pcie_write_attr(pcie, 15, (u16)PCI_MEMORY_RANGE_MASK);
> + nwl_pcie_write_attr(pcie, 16, (u16)PCI_MEMORY_RANGE_MASK);
> + /* Enable 64-bit prefetchable window */
> + nwl_pcie_write_attr(pcie, 17,
> + (u16)PCI_PREF_RANGE_MASK | PCI_PREF_RANGE_TYPE_64);
> + nwl_pcie_write_attr(pcie, 18,
> + (u16)PCI_PREF_RANGE_MASK | PCI_PREF_RANGE_TYPE_64);
> + nwl_pcie_modify_attr(pcie, 101, 0, PCIE_ATTR101_DISABLE_BAR_FILTERING);
> +
> + /* Set class code to PCI_CLASS_BRIDGE_PCI_NORMAL */
> + nwl_pcie_write_attr(pcie, 24, PCI_CLASS_BRIDGE_PCI_NORMAL & 0xffff);
> + nwl_pcie_modify_attr(pcie, 25, PCIE_ATTR25_CLASS_CODE,
> + PCIE_ATTR25_CPL_TIMEOUT_DISABLE_SUPPORTED |
> + PCI_BASE_CLASS_BRIDGE);
> +
> + /* Enable PCIe capability */
> + nwl_pcie_modify_attr(pcie, 53, PCIE_ATTR53_CAP_NEXTPTR, 0x60);
> + nwl_pcie_modify_attr(pcie, 50,
> + PCIE_ATTR50_CAP_NEXTPTR |
> + PCIE_ATTR50_CAP_DEVICE_PORT_TYPE,
> + FIELD_PREP(PCIE_ATTR50_CAP_DEVICE_PORT_TYPE,
> + PCI_EXP_TYPE_ROOT_PORT));
> +
> + /* Disable MSI(-X) capability */
> + nwl_pcie_write_attr(pcie, 41, 0);
> + nwl_pcie_write_attr(pcie, 43, 0);
> + nwl_pcie_write_attr(pcie, 44, 0);
> + nwl_pcie_write_attr(pcie, 45, 0);
> + nwl_pcie_write_attr(pcie, 46, 0);
> + nwl_pcie_write_attr(pcie, 48, 0);
> +
> + /* Disable DSN capability */
> + nwl_pcie_write_attr(pcie, 31, 0);
> + nwl_pcie_write_attr(pcie, 82, PCI_CFG_SPACE_SIZE);
> +
> + /* Enable AER */
> + nwl_pcie_modify_attr(pcie, 2, 0,
> + PCIE_ATTR2_AER_CAP_PERMIT_ROOTERR_UPDATE);
> +
> + /* Disable L1 latency for root port */
> + nwl_pcie_modify_attr(pcie, 27,
> + PCIE_ATTR27_DEV_CAP_ENDPOINT_L1_LATENCY, 0);
> +
> + /* Enable bandwidth notification */
> + nwl_pcie_modify_attr(pcie, 37, 0,
> + PCIE_ATTR37_LINK_CAP_BANDWIDTH_NOTIFICATION);
> +
> + /* Set max link width */
> + nwl_pcie_write_attr(pcie, 97,
> + FIELD_PREP(PCIE_ATTR97_LTSSM_MAX_LINK_WIDTH, width) |
> + FIELD_PREP(PCIE_ATTR97_LINK_CAP_MAX_LINK_WIDTH, width));
> +
> + /* Route misc. TLPs to controller */
> + nwl_pcie_modify_attr(pcie, 101, PCIE_ATTR101_EN_MSG_ROUTE,
> + PCIE_ATTR101_EN_MSG_ROUTE_INTA |
> + PCIE_ATTR101_EN_MSG_ROUTE_INTB |
> + PCIE_ATTR101_EN_MSG_ROUTE_INTC |
> + PCIE_ATTR101_EN_MSG_ROUTE_INTD |
> + PCIE_ATTR101_EN_MSG_ROUTE_PM_PME |
> + PCIE_ATTR101_EN_MSG_ROUTE_PME_TO_ACK |
> + PCIE_ATTR101_EN_MSG_ROUTE_UNLOCK |
> + PCIE_ATTR101_EN_MSG_ROUTE_PME_TURN_OFF);
> +
> + /* Initialize completion credits */
> + nwl_pcie_write_attr(pcie, 105, 205); /* CD */
> + nwl_pcie_write_attr(pcie, 106,
> + FIELD_PREP(PCIE_ATTR106_VC0_TOTAL_CREDITS_NPH, 12) |
> + FIELD_PREP(PCIE_ATTR106_VC0_TOTAL_CREDITS_CH, 36));
> + nwl_pcie_write_attr(pcie, 107, 24); /* NPD */
> + nwl_pcie_write_attr(pcie, 108, 181); /* PD */
> + nwl_pcie_modify_attr(pcie, 109, PCIE_ATTR109_VC0_TOTAL_CREDITS_PH, 32);
> +}
> +
> static inline u32 nwl_bridge_readl(struct nwl_pcie *pcie, u32 off)
> {
> return readl(pcie->breg_base + off);
> @@ -806,6 +965,9 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> irq_set_chained_handler_and_data(pcie->irq_intx,
> nwl_pcie_leg_handler, pcie);
>
> + pcie->ctrl_reset = devm_reset_control_get_optional(dev, "ctrl");
> + if (IS_ERR(pcie->ctrl_reset))
> + return PTR_ERR(pcie->ctrl_reset);
>
> for (i = 0; i < ARRAY_SIZE(pcie->phy); i++) {
> pcie->phy[i] = devm_of_phy_get_by_index(dev, dev->of_node, i);
> @@ -852,6 +1014,12 @@ static int nwl_pcie_probe(struct platform_device *pdev)
> if (IS_ERR(pcie->clk))
> return PTR_ERR(pcie->clk);
>
> + err = reset_control_assert(pcie->ctrl_reset);
> + if (err) {
> + dev_err(dev, "could not enter reset\n");
> + return err;
> + }
> +
> err = clk_prepare_enable(pcie->clk);
> if (err) {
> dev_err(dev, "can't enable PCIe ref clock\n");
> @@ -864,6 +1032,15 @@ static int nwl_pcie_probe(struct platform_device *pdev)
> goto err_clk;
> }
>
> + if (pcie->ctrl_reset)
> + nwl_pcie_attr_init(pcie);
> +
> + err = reset_control_deassert(pcie->ctrl_reset);
> + if (err) {
> + dev_err(dev, "could not release from reset\n");
> + goto err_phy_init;
> + }
> +
> err = nwl_pcie_phy_power_on(pcie);
> if (err) {
> dev_err(dev, "could not power on PHYs\n");
> --
> 2.35.1.1320.gc452695387.dirty
>

--
மணிவண்ணன் சதாசிவம்