On Mon, Feb 26, 2024 at 12:18:18PM -0500, Frank Li wrote: > On Sat, Feb 24, 2024 at 12:24:14PM +0530, Manivannan Sadhasivam wrote: > > The PCIe link can go to LINK_DOWN state in one of the following scenarios: > > > > 1. Fundamental (PERST#)/hot/warm reset > > 2. Link transition from L2/L3 to L0 > > From L0 to L2/l3 > I don't understand what you mean here. Link down won't happen while moving from L0 to L2/L3, it is the opposite. > > > > In those cases, LINK_DOWN causes some non-sticky DWC registers to loose the > > state (like REBAR, PTM_CAP etc...). So the drivers need to reinitialize > > them to function properly once the link comes back again. > > > > This is not a problem for drivers supporting PERST# IRQ, since they can > > reinitialize the registers in the PERST# IRQ callback. But for the drivers > > not supporting PERST#, there is no way they can reinitialize the registers > > other than relying on LINK_DOWN IRQ received when the link goes down. So > > let's add a DWC generic API dw_pcie_ep_linkdown() that reinitializes the > > non-sticky registers and also notifies the EPF drivers about link going > > down. > > > > This API can also be used by the drivers supporting PERST# to handle the > > scenario (2) mentioned above. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > > --- > > drivers/pci/controller/dwc/pcie-designware-ep.c | 111 ++++++++++++++---------- > > drivers/pci/controller/dwc/pcie-designware.h | 5 ++ > > 2 files changed, 72 insertions(+), 44 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index 278bdc9b2269..fed4c2936c78 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > @@ -14,14 +14,6 @@ > > #include <linux/pci-epc.h> > > #include <linux/pci-epf.h> > > > > -void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > > -{ > > - struct pci_epc *epc = ep->epc; > > - > > - pci_epc_linkup(epc); > > -} > > -EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup); > > - > > No sure why git remove this block and add these back. > Because, we are adding dw_pcie_ep_linkdown() API way below and it makes sense to move this API also to keep it ordered. Maybe I should've described it in commit message. - Mani > > > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep) > > { > > struct pci_epc *epc = ep->epc; > > @@ -603,19 +595,56 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap) > > return 0; > > } > > > > +static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) > > +{ > > + unsigned int offset, ptm_cap_base; > > + unsigned int nbars; > > + u32 reg, i; > > + > > + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); > > + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM); > > + > > + dw_pcie_dbi_ro_wr_en(pci); > > + > > + if (offset) { > > + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); > > + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >> > > + PCI_REBAR_CTRL_NBAR_SHIFT; > > + > > + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL) > > + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0); > > + } > > + > > + /* > > + * PTM responder capability can be disabled only after disabling > > + * PTM root capability. > > + */ > > + if (ptm_cap_base) { > > + dw_pcie_dbi_ro_wr_en(pci); > > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > > + reg &= ~PCI_PTM_CAP_ROOT; > > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > > + > > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > > + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK); > > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > > + dw_pcie_dbi_ro_wr_dis(pci); > > + } > > + > > + dw_pcie_setup(pci); > > + dw_pcie_dbi_ro_wr_dis(pci); > > +} > > + > > int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep) > > { > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > > struct dw_pcie_ep_func *ep_func; > > struct device *dev = pci->dev; > > struct pci_epc *epc = ep->epc; > > - unsigned int offset, ptm_cap_base; > > - unsigned int nbars; > > u8 hdr_type; > > u8 func_no; > > - int i, ret; > > void *addr; > > - u32 reg; > > + int ret; > > > > hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) & > > PCI_HEADER_TYPE_MASK; > > @@ -678,38 +707,7 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep) > > if (ep->ops->init) > > ep->ops->init(ep); > > > > - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); > > - ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM); > > - > > - dw_pcie_dbi_ro_wr_en(pci); > > - > > - if (offset) { > > - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); > > - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >> > > - PCI_REBAR_CTRL_NBAR_SHIFT; > > - > > - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL) > > - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0); > > - } > > - > > - /* > > - * PTM responder capability can be disabled only after disabling > > - * PTM root capability. > > - */ > > - if (ptm_cap_base) { > > - dw_pcie_dbi_ro_wr_en(pci); > > - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > > - reg &= ~PCI_PTM_CAP_ROOT; > > - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > > - > > - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > > - reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK); > > - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > > - dw_pcie_dbi_ro_wr_dis(pci); > > - } > > - > > - dw_pcie_setup(pci); > > - dw_pcie_dbi_ro_wr_dis(pci); > > + dw_pcie_ep_init_non_sticky_registers(pci); > > > > return 0; > > > > @@ -720,6 +718,31 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep) > > } > > EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers); > > > > +void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > > +{ > > + struct pci_epc *epc = ep->epc; > > + > > + pci_epc_linkup(epc); > > +} > > +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup); > > + > > +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep) > > +{ > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > > + struct pci_epc *epc = ep->epc; > > + > > + /* > > + * Initialize the non-sticky DWC registers as they would've reset post > > + * LINK_DOWN. This is specifically needed for drivers not supporting > > + * PERST# as they have no way to reinitialize the registers before the > > + * link comes back again. > > + */ > > + dw_pcie_ep_init_non_sticky_registers(pci); > > + > > + pci_epc_linkdown(epc); > > +} > > +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown); > > + > > int dw_pcie_ep_init(struct dw_pcie_ep *ep) > > { > > int ret; > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > > index f8e5431a207b..152969545b0a 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > @@ -668,6 +668,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, > > > > #ifdef CONFIG_PCIE_DW_EP > > void dw_pcie_ep_linkup(struct dw_pcie_ep *ep); > > +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep); > > int dw_pcie_ep_init(struct dw_pcie_ep *ep); > > int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep); > > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep); > > @@ -688,6 +689,10 @@ static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > > { > > } > > > > +static inline void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep) > > +{ > > +} > > + > > static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep) > > { > > return 0; > > > > -- > > 2.25.1 > > -- மணிவண்ணன் சதாசிவம்