| Message ID | 20240224-pci-dbi-rework-v8-8-64c7fd0cfe64@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: dwc: ep: Fix DBI access failure for drivers requiring refclk from host | expand |
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
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@linaro.org> > > --- > > 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 > >
On Tue, Feb 27, 2024 at 06:00:24PM +0530, Manivannan Sadhasivam wrote: > 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. Strange, why there are not linkdown from L0 to L2/l3. But have linkdown from L2/l3 to L0? when linkup happen? Any document show these? Frank > > > > > > > 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@linaro.org> > > > --- > > > 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 > > > > > -- > மணிவண்ணன் சதாசிவம்
On Tue, Feb 27, 2024 at 12:26:05PM -0500, Frank Li wrote: > On Tue, Feb 27, 2024 at 06:00:24PM +0530, Manivannan Sadhasivam wrote: > > 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. > > Strange, why there are not linkdown from L0 to L2/l3. But have linkdown > from L2/l3 to L0? when linkup happen? Any document show these? > Refer PCIe Spec 5.0, Figure 5-1 Link Power Management State Flow Diagram. - Mani > Frank > > > > > > > > > > > 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@linaro.org> > > > > --- > > > > 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 > > > > > > > > -- > > மணிவண்ணன் சதாசிவம்
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); - 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;
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 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@linaro.org> --- drivers/pci/controller/dwc/pcie-designware-ep.c | 111 ++++++++++++++---------- drivers/pci/controller/dwc/pcie-designware.h | 5 ++ 2 files changed, 72 insertions(+), 44 deletions(-)