* [PATCH 1/2] pci/layercape: disable all iATUs before initialization @ 2016-09-08 6:25 Minghuan Lian 2016-09-08 6:25 ` [PATCH 2/2] pci/layerscape: fix error response behavior Minghuan Lian 2016-09-14 21:10 ` [PATCH 1/2] pci/layercape: disable all iATUs before initialization Bjorn Helgaas 0 siblings, 2 replies; 4+ messages in thread From: Minghuan Lian @ 2016-09-08 6:25 UTC (permalink / raw) To: linux-pci Cc: Roy Zang, Arnd Bergmann, Jingoo Han, Stuart Yoder, Yang-Leo Li, Minghuan Lian, linux-arm-kernel, Bjorn Helgaas, Mingkai Hu Layerscape PCIe has 6 outbound iATUs. The bootloader such as u-boot uses 4 iATUs for CFG0 CFG1 IO and MEM separately. But Designware driver only uses two outbound iATUs. To avoid conflict between enabled but unused iATUs with used iATUs under Linux and unexpected behavior, the patch disables all iATUs before initialization. Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com> --- drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++-- drivers/pci/host/pcie-designware.c | 7 +++++++ drivers/pci/host/pcie-designware.h | 1 + 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c index 114ba81..cf783ad 100644 --- a/drivers/pci/host/pci-layerscape.c +++ b/drivers/pci/host/pci-layerscape.c @@ -38,6 +38,8 @@ /* PEX LUT registers */ #define PCIE_LUT_DBG 0x7FC /* PEX LUT Debug Register */ +#define PCIE_IATU_NUM 6 + struct ls_pcie_drvdata { u32 lut_offset; u32 ltssm_shift; @@ -55,6 +57,8 @@ struct ls_pcie { #define to_ls_pcie(x) container_of(x, struct ls_pcie, pp) +static void ls_pcie_host_init(struct pcie_port *pp); + static bool ls_pcie_is_bridge(struct ls_pcie *pcie) { u32 header_type; @@ -87,6 +91,14 @@ static void ls_pcie_drop_msg_tlp(struct ls_pcie *pcie) iowrite32(val, pcie->dbi + PCIE_STRFMR1); } +static void ls_pcie_disable_outbound_atus(struct ls_pcie *pcie) +{ + int i; + + for (i = 0; i < PCIE_IATU_NUM; i++) + dw_pcie_disable_outbound_atu(&pcie->pp, i); +} + static int ls1021_pcie_link_up(struct pcie_port *pp) { u32 state; @@ -124,9 +136,8 @@ static void ls1021_pcie_host_init(struct pcie_port *pp) } pcie->index = index[1]; + ls_pcie_host_init(pp); dw_pcie_setup_rc(pp); - - ls_pcie_drop_msg_tlp(pcie); } static int ls_pcie_link_up(struct pcie_port *pp) @@ -153,6 +164,8 @@ static void ls_pcie_host_init(struct pcie_port *pp) ls_pcie_clear_multifunction(pcie); ls_pcie_drop_msg_tlp(pcie); iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN); + + ls_pcie_disable_outbound_atus(pcie); } static int ls_pcie_msi_host_init(struct pcie_port *pp, diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index 12afce1..e4d1203 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -172,6 +172,13 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index, dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val); } +void dw_pcie_disable_outbound_atu(struct pcie_port *pp, int index) +{ + dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index, + PCIE_ATU_VIEWPORT); + dw_pcie_writel_rc(pp, 0, PCIE_ATU_CR2); +} + static struct irq_chip dw_msi_irq_chip = { .name = "PCI-MSI", .irq_enable = pci_msi_unmask_irq, diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h index f437f9b..e998bfc 100644 --- a/drivers/pci/host/pcie-designware.h +++ b/drivers/pci/host/pcie-designware.h @@ -85,5 +85,6 @@ int dw_pcie_wait_for_link(struct pcie_port *pp); int dw_pcie_link_up(struct pcie_port *pp); void dw_pcie_setup_rc(struct pcie_port *pp); int dw_pcie_host_init(struct pcie_port *pp); +void dw_pcie_disable_outbound_atu(struct pcie_port *pp, int index); #endif /* _PCIE_DESIGNWARE_H */ -- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] pci/layerscape: fix error response behavior 2016-09-08 6:25 [PATCH 1/2] pci/layercape: disable all iATUs before initialization Minghuan Lian @ 2016-09-08 6:25 ` Minghuan Lian 2016-09-14 21:10 ` [PATCH 1/2] pci/layercape: disable all iATUs before initialization Bjorn Helgaas 1 sibling, 0 replies; 4+ messages in thread From: Minghuan Lian @ 2016-09-08 6:25 UTC (permalink / raw) To: linux-pci Cc: Roy Zang, Arnd Bergmann, Jingoo Han, Stuart Yoder, Yang-Leo Li, Minghuan Lian, linux-arm-kernel, Bjorn Helgaas, Mingkai Hu By default, when the PCIe controller experiences an erroneous completion from an external completer for its outbound non-posted request, it always sends an OKAY response to the device's internal AXI slave system interface. However, such default system error response behavior cannot be used for other types of outbound non-posted requests. For example, the outbound memory read transaction requires an actual ERROR response, like UR completion or completion timeout. The patch is to fix it by forwarding the error response of the non-posted request. Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com> --- drivers/pci/host/pci-layerscape.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c index cf783ad..873447a 100644 --- a/drivers/pci/host/pci-layerscape.c +++ b/drivers/pci/host/pci-layerscape.c @@ -34,6 +34,8 @@ /* PEX Internal Configuration Registers */ #define PCIE_STRFMR1 0x71c /* Symbol Timer & Filter Mask Register1 */ #define PCIE_DBI_RO_WR_EN 0x8bc /* DBI Read-Only Write Enable Register */ +#define PCIE_ABSERR 0x8d0 /* Bridge Slave Error Response Register */ +#define PCIE_ABSERR_SETTING 0x9401 /* Forward error of non-posted request */ /* PEX LUT registers */ #define PCIE_LUT_DBG 0x7FC /* PEX LUT Debug Register */ @@ -99,6 +101,12 @@ static void ls_pcie_disable_outbound_atus(struct ls_pcie *pcie) dw_pcie_disable_outbound_atu(&pcie->pp, i); } +/* Forward error response of outbound non-posted requests */ +static void ls_pcie_fix_error_response(struct ls_pcie *pcie) +{ + iowrite32(PCIE_ABSERR_SETTING, pcie->dbi + PCIE_ABSERR); +} + static int ls1021_pcie_link_up(struct pcie_port *pp) { u32 state; @@ -166,6 +174,7 @@ static void ls_pcie_host_init(struct pcie_port *pp) iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN); ls_pcie_disable_outbound_atus(pcie); + ls_pcie_fix_error_response(pcie); } static int ls_pcie_msi_host_init(struct pcie_port *pp, -- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] pci/layercape: disable all iATUs before initialization 2016-09-08 6:25 [PATCH 1/2] pci/layercape: disable all iATUs before initialization Minghuan Lian 2016-09-08 6:25 ` [PATCH 2/2] pci/layerscape: fix error response behavior Minghuan Lian @ 2016-09-14 21:10 ` Bjorn Helgaas 2016-09-20 7:25 ` M.H. Lian 1 sibling, 1 reply; 4+ messages in thread From: Bjorn Helgaas @ 2016-09-14 21:10 UTC (permalink / raw) To: Minghuan Lian Cc: linux-pci, Roy Zang, Arnd Bergmann, Jingoo Han, Stuart Yoder, Yang-Leo Li, linux-arm-kernel, Bjorn Helgaas, Mingkai Hu On Thu, Sep 08, 2016 at 02:25:49PM +0800, Minghuan Lian wrote: > Layerscape PCIe has 6 outbound iATUs. The bootloader such as > u-boot uses 4 iATUs for CFG0 CFG1 IO and MEM separately. But > Designware driver only uses two outbound iATUs. To avoid > conflict between enabled but unused iATUs with used iATUs > under Linux and unexpected behavior, the patch disables all > iATUs before initialization. Do we need similar changes in other DesignWare-based drivers? > Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com> > --- > drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++-- > drivers/pci/host/pcie-designware.c | 7 +++++++ > drivers/pci/host/pcie-designware.h | 1 + > 3 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c > index 114ba81..cf783ad 100644 > --- a/drivers/pci/host/pci-layerscape.c > +++ b/drivers/pci/host/pci-layerscape.c > @@ -38,6 +38,8 @@ > /* PEX LUT registers */ > #define PCIE_LUT_DBG 0x7FC /* PEX LUT Debug Register */ > > +#define PCIE_IATU_NUM 6 > + > struct ls_pcie_drvdata { > u32 lut_offset; > u32 ltssm_shift; > @@ -55,6 +57,8 @@ struct ls_pcie { > > #define to_ls_pcie(x) container_of(x, struct ls_pcie, pp) > > +static void ls_pcie_host_init(struct pcie_port *pp); I would prefer to reorder the function definitions so the forward declaration is not necessary. This would be two patches: 1) Reorder functions (no functional change) 2) Disable iATUs > static bool ls_pcie_is_bridge(struct ls_pcie *pcie) > { > u32 header_type; > @@ -87,6 +91,14 @@ static void ls_pcie_drop_msg_tlp(struct ls_pcie *pcie) > iowrite32(val, pcie->dbi + PCIE_STRFMR1); > } > > +static void ls_pcie_disable_outbound_atus(struct ls_pcie *pcie) > +{ > + int i; > + > + for (i = 0; i < PCIE_IATU_NUM; i++) > + dw_pcie_disable_outbound_atu(&pcie->pp, i); It looks like maybe the DesignWare ATUs are generic enough that we could move the loop into the generic code, e.g., void dw_pcie_disable_outbound_atu(struct pcie_port *pp, int count) { int i; for (i = 0; i < count; i++) { dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | i, PCIE_ATU_VIEWPORT); dw_pcie_writel_rc(pp, 0, PCIE_ATU_CR2); } } > +} > + > static int ls1021_pcie_link_up(struct pcie_port *pp) > { > u32 state; > @@ -124,9 +136,8 @@ static void ls1021_pcie_host_init(struct pcie_port *pp) > } > pcie->index = index[1]; > > + ls_pcie_host_init(pp); > dw_pcie_setup_rc(pp); > - > - ls_pcie_drop_msg_tlp(pcie); Can you split this to a separate patch? This hunk changes ls1021_pcie_host_init() so it does several things in addition to ls_pcie_drop_msg_tlp(), and it is unrelated to the "disable iATU" change. > } > > static int ls_pcie_link_up(struct pcie_port *pp) > @@ -153,6 +164,8 @@ static void ls_pcie_host_init(struct pcie_port *pp) > ls_pcie_clear_multifunction(pcie); > ls_pcie_drop_msg_tlp(pcie); > iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN); > + > + ls_pcie_disable_outbound_atus(pcie); > } > > static int ls_pcie_msi_host_init(struct pcie_port *pp, > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index 12afce1..e4d1203 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -172,6 +172,13 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index, > dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val); > } > > +void dw_pcie_disable_outbound_atu(struct pcie_port *pp, int index) > +{ > + dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index, > + PCIE_ATU_VIEWPORT); > + dw_pcie_writel_rc(pp, 0, PCIE_ATU_CR2); > +} > + > static struct irq_chip dw_msi_irq_chip = { > .name = "PCI-MSI", > .irq_enable = pci_msi_unmask_irq, > diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h > index f437f9b..e998bfc 100644 > --- a/drivers/pci/host/pcie-designware.h > +++ b/drivers/pci/host/pcie-designware.h > @@ -85,5 +85,6 @@ int dw_pcie_wait_for_link(struct pcie_port *pp); > int dw_pcie_link_up(struct pcie_port *pp); > void dw_pcie_setup_rc(struct pcie_port *pp); > int dw_pcie_host_init(struct pcie_port *pp); > +void dw_pcie_disable_outbound_atu(struct pcie_port *pp, int index); > > #endif /* _PCIE_DESIGNWARE_H */ > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH 1/2] pci/layercape: disable all iATUs before initialization 2016-09-14 21:10 ` [PATCH 1/2] pci/layercape: disable all iATUs before initialization Bjorn Helgaas @ 2016-09-20 7:25 ` M.H. Lian 0 siblings, 0 replies; 4+ messages in thread From: M.H. Lian @ 2016-09-20 7:25 UTC (permalink / raw) To: Bjorn Helgaas Cc: Roy Zang, Arnd Bergmann, linux-pci@vger.kernel.org, Jingoo Han, Stuart Yoder, Leo Li, Mingkai Hu, Bjorn Helgaas, linux-arm-kernel@lists.infradead.org Hi Bjorn, Thanks for your reply. Please see my comments inline. Best Regards, Minghuan > -----Original Message----- > From: Bjorn Helgaas [mailto:helgaas@kernel.org] > Sent: Thursday, September 15, 2016 5:11 AM > To: M.H. Lian <minghuan.lian@nxp.com> > Cc: linux-pci@vger.kernel.org; Roy Zang <roy.zang@nxp.com>; Arnd > Bergmann <arnd@arndb.de>; Jingoo Han <jg1.han@samsung.com>; Stuart > Yoder <stuart.yoder@nxp.com>; Leo Li <leoyang.li@nxp.com>; linux-arm- > kernel@lists.infradead.org; Bjorn Helgaas <bhelgaas@google.com>; Mingkai > Hu <mingkai.hu@nxp.com> > Subject: Re: [PATCH 1/2] pci/layercape: disable all iATUs before initialization > > On Thu, Sep 08, 2016 at 02:25:49PM +0800, Minghuan Lian wrote: > > Layerscape PCIe has 6 outbound iATUs. The bootloader such as u-boot > > uses 4 iATUs for CFG0 CFG1 IO and MEM separately. But Designware > > driver only uses two outbound iATUs. To avoid conflict between enabled > > but unused iATUs with used iATUs under Linux and unexpected behavior, > > the patch disables all iATUs before initialization. > > Do we need similar changes in other DesignWare-based drivers? [Minghuan Lian] Yes. I think so. I could provide a patch for all the Designware-based drivers. Could you give me suggestion how to get ATU number of all kinds of PCIe controller? 1. Add optional "num-atu" property to designware-based PCIe dts node? 2. The specific driver assign a variable "num-atu" before calling dw_pcie_host_init() We could achieve benefits if the ATU number is bigger than two (current default value) 1. A dedicated ATU is for IO map. It will avoid conflict IO and config access simultaneously. 2. Supports multiple ATUs for prefetchable and non-prefetchable memory and the memory size can be greater than 4G. > > > Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com> > > --- > > drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++-- > > drivers/pci/host/pcie-designware.c | 7 +++++++ > > drivers/pci/host/pcie-designware.h | 1 + > > 3 files changed, 23 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/host/pci-layerscape.c > > b/drivers/pci/host/pci-layerscape.c > > index 114ba81..cf783ad 100644 > > --- a/drivers/pci/host/pci-layerscape.c > > +++ b/drivers/pci/host/pci-layerscape.c > > @@ -38,6 +38,8 @@ > > /* PEX LUT registers */ > > #define PCIE_LUT_DBG 0x7FC /* PEX LUT Debug Register */ > > > > +#define PCIE_IATU_NUM 6 > > + > > struct ls_pcie_drvdata { > > u32 lut_offset; > > u32 ltssm_shift; > > @@ -55,6 +57,8 @@ struct ls_pcie { > > > > #define to_ls_pcie(x) container_of(x, struct ls_pcie, pp) > > > > +static void ls_pcie_host_init(struct pcie_port *pp); > > I would prefer to reorder the function definitions so the forward declaration > is not necessary. This would be two patches: > > 1) Reorder functions (no functional change) > 2) Disable iATUs [Minghuan Lian] ok. I will change it. > > > static bool ls_pcie_is_bridge(struct ls_pcie *pcie) { > > u32 header_type; > > @@ -87,6 +91,14 @@ static void ls_pcie_drop_msg_tlp(struct ls_pcie *pcie) > > iowrite32(val, pcie->dbi + PCIE_STRFMR1); } > > > > +static void ls_pcie_disable_outbound_atus(struct ls_pcie *pcie) { > > + int i; > > + > > + for (i = 0; i < PCIE_IATU_NUM; i++) > > + dw_pcie_disable_outbound_atu(&pcie->pp, i); > > It looks like maybe the DesignWare ATUs are generic enough that we could > move the loop into the generic code, e.g., > > void dw_pcie_disable_outbound_atu(struct pcie_port *pp, int count) > { > int i; > > for (i = 0; i < count; i++) { > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | i, > PCIE_ATU_VIEWPORT); > dw_pcie_writel_rc(pp, 0, PCIE_ATU_CR2); > } > } > [Minghuan Lian] Yes. And, If we add "num-atu" property to dts node. Designware driver can directly clear ATUs and the specific driver needs to do nothing. > > + > > static int ls1021_pcie_link_up(struct pcie_port *pp) { > > u32 state; > > @@ -124,9 +136,8 @@ static void ls1021_pcie_host_init(struct pcie_port > *pp) > > } > > pcie->index = index[1]; > > > > + ls_pcie_host_init(pp); > > dw_pcie_setup_rc(pp); > > - > > - ls_pcie_drop_msg_tlp(pcie); > > Can you split this to a separate patch? This hunk changes > ls1021_pcie_host_init() so it does several things in addition to > ls_pcie_drop_msg_tlp(), and it is unrelated to the "disable iATU" change. [Minghuan Lian] OK. I will change it. > > > } > > > > static int ls_pcie_link_up(struct pcie_port *pp) @@ -153,6 +164,8 @@ > > static void ls_pcie_host_init(struct pcie_port *pp) > > ls_pcie_clear_multifunction(pcie); > > ls_pcie_drop_msg_tlp(pcie); > > iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN); > > + > > + ls_pcie_disable_outbound_atus(pcie); > > } > > > > static int ls_pcie_msi_host_init(struct pcie_port *pp, diff --git > > a/drivers/pci/host/pcie-designware.c > > b/drivers/pci/host/pcie-designware.c > > index 12afce1..e4d1203 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -172,6 +172,13 @@ static void dw_pcie_prog_outbound_atu(struct > pcie_port *pp, int index, > > dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val); } > > > > +void dw_pcie_disable_outbound_atu(struct pcie_port *pp, int index) { > > + dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index, > > + PCIE_ATU_VIEWPORT); > > + dw_pcie_writel_rc(pp, 0, PCIE_ATU_CR2); } > > + > > static struct irq_chip dw_msi_irq_chip = { > > .name = "PCI-MSI", > > .irq_enable = pci_msi_unmask_irq, > > diff --git a/drivers/pci/host/pcie-designware.h > > b/drivers/pci/host/pcie-designware.h > > index f437f9b..e998bfc 100644 > > --- a/drivers/pci/host/pcie-designware.h > > +++ b/drivers/pci/host/pcie-designware.h > > @@ -85,5 +85,6 @@ int dw_pcie_wait_for_link(struct pcie_port *pp); > > int dw_pcie_link_up(struct pcie_port *pp); void > > dw_pcie_setup_rc(struct pcie_port *pp); int dw_pcie_host_init(struct > > pcie_port *pp); > > +void dw_pcie_disable_outbound_atu(struct pcie_port *pp, int index); > > > > #endif /* _PCIE_DESIGNWARE_H */ > > -- > > 1.9.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-20 7:25 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-08 6:25 [PATCH 1/2] pci/layercape: disable all iATUs before initialization Minghuan Lian 2016-09-08 6:25 ` [PATCH 2/2] pci/layerscape: fix error response behavior Minghuan Lian 2016-09-14 21:10 ` [PATCH 1/2] pci/layercape: disable all iATUs before initialization Bjorn Helgaas 2016-09-20 7:25 ` M.H. Lian
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).