pci/layercape: disable all iATUs before initialization
PATCH 1/2 pci/layercape: disable all iATUs before initialization
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.
--- 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(-)
drivers/pci/host/pci-layerscape.c +15 -2
@@ -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,
drivers/pci/host/pcie-designware.c +7 -0
@@ -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,
drivers/pci/host/pcie-designware.h +1 -0
@@ -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 */
Signature · 1 line
1.9.1
PATCH 2/2 pci/layerscape: fix error response behavior
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.
--- drivers/pci/host/pci-layerscape.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/pci/host/pci-layerscape.c +9 -0
@@ -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,
Signature · 1 line
1.9.1
PATCH 1/2 pci/layercape: disable all iATUs before initialization
In reply to Minghuan LianQuoted 6 lines · Minghuan Lian
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?
Quoted 21 lines
Signed-off-by Minghuan Lian <[email protected]>--- 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(-)drivers/pci/host/pci-layerscape.c +2
-0@@ -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;
drivers/pci/host/pci-layerscape.c struct ls_pcie { · line 60
Quoted 4 lines
drivers/pci/host/pci-layerscape.c +1
-0@@ -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
drivers/pci/host/pci-layerscape.c static void ls_pcie_drop_msg_tlp(struct ls_pcie *pcie) · line 99
Quoted 13 lines
drivers/pci/host/pci-layerscape.c +6
-0static 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);
}}
drivers/pci/host/pci-layerscape.c static void ls1021_pcie_host_init(struct pcie_port *pp) · line 129
Quoted 13 lines
drivers/pci/host/pci-layerscape.c +3
-2+} +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.
Quoted 49 lines
drivers/pci/host/pci-layerscape.c +2
-0}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,drivers/pci/host/pcie-designware.c +7
-0@@ -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,drivers/pci/host/pcie-designware.h +1
-0@@ -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 */ Signature · 7 lines
1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/2 pci/layercape: disable all iATUs before initialization
Hi Bjorn,
Thanks for your reply.
Please see my comments inline.
Best Regards,
Minghuan
Quoted 11 lines
-----Original Message-----
From: Bjorn Helgaas [mailto:helgaas at kernel.org]
Sent: Thursday, September 15, 2016 5:11 AM
To: M.H. Lian <[email protected]>
Cc: linux-pci at vger.kernel.org; Roy Zang <[email protected]>; Arnd Bergmann <[email protected]>; Jingoo Han <[email protected]>; Stuart Yoder <[email protected]>; Leo Li <[email protected]>; linux-arm- kernel at lists.infradead.org; Bjorn Helgaas <[email protected]>; Mingkai Hu <[email protected]>
Subject: Re: [PATCH 1/2] pci/layercape: disable all iATUs before initialization
Quoted 8 lines
Quoted 5 lines · Minghuan Lian
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.
Quoted 26 lines
Quoted 26 lines
Signed-off-by Minghuan Lian <[email protected]>--- 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(-)drivers/pci/host/pci-layerscape.c +2
-0@@ -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)
Quoted 7 lines
drivers/pci/host/pci-layerscape.c struct ls_pcie { · line 60
Quoted 1 line
drivers/pci/host/pci-layerscape.c +1
-0+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.
Quoted 19 lines
drivers/pci/host/pci-layerscape.c static void ls_pcie_drop_msg_tlp(struct ls_pcie *pcie) · line 97
Quoted 11 lines
drivers/pci/host/pci-layerscape.c +5
-0static 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;
Quoted 7 lines
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.
Quoted 8 lines
drivers/pci/host/pci-layerscape.c static void ls1021_pcie_host_init(struct pcie_port
Quoted 4 lines
drivers/pci/host/pci-layerscape.c +1
-0+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)
Quoted 3 lines
drivers/pci/host/pci-layerscape.c +0
-0} pcie->index = index[1 ];
Quoted 8 lines
drivers/pci/host/pci-layerscape.c static void ls1021_pcie_host_init(struct pcie_port *pp) · line 129
Quoted 4 lines
drivers/pci/host/pci-layerscape.c +1
-2+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 tols_pcie_drop_msg_tlp(), and it is unrelated to the "disable iATU" change.
[Minghuan Lian] OK. I will change it.
Quoted 50 lines
drivers/pci/host/pcie-designware.c static void dw_pcie_prog_outbound_atu(struct
Quoted 19 lines
drivers/pci/host/pci-layerscape.c +0
-0} static int ls_pcie_link_up(struct pcie_port *pp) @@ -153,6 +164,8 @@static void
ls_pcie_host_init(struct pcie_port *pp)drivers/pci/host/pci-layerscape.c +2
-0ls_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.cb/drivers/pci/host/pcie-designware.c
index 12afce1..e4d1203 100644drivers/pci/host/pcie-designware.c +0
-0@@ -172,6 +172,13 @@ static void dw_pcie_prog_outbound_atu(struct
pcie_port*pp, int index,Quoted 30 lines
drivers/pci/host/pcie-designware.c +5
-0dw_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,drivers/pci/host/pcie-designware.h +0
-0@@ -85,5 +85,6 @@ int dw_pcie_wait_for_link(struct pcie_port *pp);int
dw_pcie_link_up(struct pcie_port *pp); voiddw_pcie_setup_rc(struct pcie_port *pp); intdw_pcie_host_init(struct pcie_port *pp);
+voiddw_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 at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel