All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] PCI: layerscape: add fixes for layerscape-pcie errata
@ 2017-09-22  7:25 ` Zhiqiang Hou
  0 siblings, 0 replies; 22+ messages in thread
From: Zhiqiang Hou @ 2017-09-22  7:25 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, roy.zang,
	mingkai.hu, minghuan.lian
  Cc: Hou Zhiqiang

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

The [1/2] is to fix layerscape PCIe MSI/MSI-X capability errata.
The [2/2] is to change the default AXI system error response behavior
for PCI Express outbound non-posted requests.

Hou Zhiqiang (1):
  PCI: Disable MSI for Freescale PCIe RC mode

Minghuan Lian (1):
  pci/layerscape: change the default error response behavior

 drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
 drivers/pci/quirks.c             |  8 ++++++++
 2 files changed, 33 insertions(+)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 0/2] PCI: layerscape: add fixes for layerscape-pcie errata
@ 2017-09-22  7:25 ` Zhiqiang Hou
  0 siblings, 0 replies; 22+ messages in thread
From: Zhiqiang Hou @ 2017-09-22  7:25 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, roy.zang,
	mingkai.hu, minghuan.lian
  Cc: Hou Zhiqiang

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

The [1/2] is to fix layerscape PCIe MSI/MSI-X capability errata.
The [2/2] is to change the default AXI system error response behavior
for PCI Express outbound non-posted requests.

Hou Zhiqiang (1):
  PCI: Disable MSI for Freescale PCIe RC mode

Minghuan Lian (1):
  pci/layerscape: change the default error response behavior

 drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
 drivers/pci/quirks.c             |  8 ++++++++
 2 files changed, 33 insertions(+)

-- 
2.14.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] 22+ messages in thread

* [PATCH 0/2] PCI: layerscape: add fixes for layerscape-pcie errata
@ 2017-09-22  7:25 ` Zhiqiang Hou
  0 siblings, 0 replies; 22+ messages in thread
From: Zhiqiang Hou @ 2017-09-22  7:25 UTC (permalink / raw)
  To: linux-arm-kernel

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

The [1/2] is to fix layerscape PCIe MSI/MSI-X capability errata.
The [2/2] is to change the default AXI system error response behavior
for PCI Express outbound non-posted requests.

Hou Zhiqiang (1):
  PCI: Disable MSI for Freescale PCIe RC mode

Minghuan Lian (1):
  pci/layerscape: change the default error response behavior

 drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
 drivers/pci/quirks.c             |  8 ++++++++
 2 files changed, 33 insertions(+)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
  2017-09-22  7:25 ` Zhiqiang Hou
@ 2017-09-22  7:25   ` Zhiqiang Hou
  -1 siblings, 0 replies; 22+ messages in thread
From: Zhiqiang Hou @ 2017-09-22  7:25 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, roy.zang,
	mingkai.hu, minghuan.lian
  Cc: Hou Zhiqiang

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

The Freescale PCIe controller advertises the MSI/MSI-X capability
in both RC and Endpoint mode, but in RC mode it doesn't support
MSI/MSI-X by it self, it can only transfer MSI/MSI-X from downstream
devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 drivers/pci/quirks.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index a4d33619a7bb..c1063a420f0c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
 /* AMD Stoney platform GPU */
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
 #endif /* CONFIG_PCI_ATS */
+
+/* Freescale PCIe doesn't support MSI in RC mode */
+static void quirk_fsl_no_msi(struct pci_dev *pdev)
+{
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
+		pdev->no_msi = 1;
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, quirk_fsl_no_msi);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
@ 2017-09-22  7:25   ` Zhiqiang Hou
  0 siblings, 0 replies; 22+ messages in thread
From: Zhiqiang Hou @ 2017-09-22  7:25 UTC (permalink / raw)
  To: linux-arm-kernel

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

The Freescale PCIe controller advertises the MSI/MSI-X capability
in both RC and Endpoint mode, but in RC mode it doesn't support
MSI/MSI-X by it self, it can only transfer MSI/MSI-X from downstream
devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 drivers/pci/quirks.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index a4d33619a7bb..c1063a420f0c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
 /* AMD Stoney platform GPU */
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
 #endif /* CONFIG_PCI_ATS */
+
+/* Freescale PCIe doesn't support MSI in RC mode */
+static void quirk_fsl_no_msi(struct pci_dev *pdev)
+{
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
+		pdev->no_msi = 1;
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, quirk_fsl_no_msi);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/2] pci/layerscape: change the default error response behavior
  2017-09-22  7:25 ` Zhiqiang Hou
@ 2017-09-22  7:25   ` Zhiqiang Hou
  -1 siblings, 0 replies; 22+ messages in thread
From: Zhiqiang Hou @ 2017-09-22  7:25 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, roy.zang,
	mingkai.hu, minghuan.lian
  Cc: Minghuan Lian, Hou Zhiqiang

From: Minghuan Lian <Minghuan.Lian@nxp.com>

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>
Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/pci/dwc/pci-layerscape.c b/drivers/pci/dwc/pci-layerscape.c
index 3b01e309a55e..a647090c140e 100644
--- a/drivers/pci/dwc/pci-layerscape.c
+++ b/drivers/pci/dwc/pci-layerscape.c
@@ -33,6 +33,8 @@
 
 /* PEX Internal Configuration Registers */
 #define PCIE_STRFMR1		0x71c /* Symbol Timer & Filter Mask Register1 */
+#define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
+#define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
 
 #define PCIE_IATU_NUM		6
 
@@ -54,6 +56,19 @@ struct ls_pcie {
 
 #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
 
+static int err_response_flag = 1;
+
+static int __init ls_pcie_param(char *p)
+{
+	if (p && strncmp(p, "no-err-response", 15) == 0)
+		err_response_flag = 0;
+	else
+		err_response_flag = 1;
+
+	return 0;
+}
+early_param("ls_pcie", ls_pcie_param);
+
 static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
 {
 	struct dw_pcie *pci = pcie->pci;
@@ -124,6 +139,14 @@ static int ls_pcie_link_up(struct dw_pcie *pci)
 	return 1;
 }
 
+/* Forward error response of outbound non-posted requests */
+static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
+{
+	struct dw_pcie *pci = pcie->pci;
+
+	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
+}
+
 static int ls_pcie_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -135,6 +158,8 @@ static int ls_pcie_host_init(struct pcie_port *pp)
 	 * dw_pcie_setup_rc() will reconfigure the outbound windows.
 	 */
 	ls_pcie_disable_outbound_atus(pcie);
+	if (err_response_flag)
+		ls_pcie_fix_error_response(pcie);
 
 	dw_pcie_dbi_ro_wr_en(pci);
 	ls_pcie_clear_multifunction(pcie);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/2] pci/layerscape: change the default error response behavior
@ 2017-09-22  7:25   ` Zhiqiang Hou
  0 siblings, 0 replies; 22+ messages in thread
From: Zhiqiang Hou @ 2017-09-22  7:25 UTC (permalink / raw)
  To: linux-arm-kernel

From: Minghuan Lian <Minghuan.Lian@nxp.com>

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>
Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/pci/dwc/pci-layerscape.c b/drivers/pci/dwc/pci-layerscape.c
index 3b01e309a55e..a647090c140e 100644
--- a/drivers/pci/dwc/pci-layerscape.c
+++ b/drivers/pci/dwc/pci-layerscape.c
@@ -33,6 +33,8 @@
 
 /* PEX Internal Configuration Registers */
 #define PCIE_STRFMR1		0x71c /* Symbol Timer & Filter Mask Register1 */
+#define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
+#define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
 
 #define PCIE_IATU_NUM		6
 
@@ -54,6 +56,19 @@ struct ls_pcie {
 
 #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
 
+static int err_response_flag = 1;
+
+static int __init ls_pcie_param(char *p)
+{
+	if (p && strncmp(p, "no-err-response", 15) == 0)
+		err_response_flag = 0;
+	else
+		err_response_flag = 1;
+
+	return 0;
+}
+early_param("ls_pcie", ls_pcie_param);
+
 static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
 {
 	struct dw_pcie *pci = pcie->pci;
@@ -124,6 +139,14 @@ static int ls_pcie_link_up(struct dw_pcie *pci)
 	return 1;
 }
 
+/* Forward error response of outbound non-posted requests */
+static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
+{
+	struct dw_pcie *pci = pcie->pci;
+
+	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
+}
+
 static int ls_pcie_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -135,6 +158,8 @@ static int ls_pcie_host_init(struct pcie_port *pp)
 	 * dw_pcie_setup_rc() will reconfigure the outbound windows.
 	 */
 	ls_pcie_disable_outbound_atus(pcie);
+	if (err_response_flag)
+		ls_pcie_fix_error_response(pcie);
 
 	dw_pcie_dbi_ro_wr_en(pci);
 	ls_pcie_clear_multifunction(pcie);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
  2017-09-22  7:25   ` Zhiqiang Hou
  (?)
@ 2017-10-11 19:37     ` Bjorn Helgaas
  -1 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2017-10-11 19:37 UTC (permalink / raw)
  To: Zhiqiang Hou
  Cc: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, roy.zang,
	mingkai.hu, minghuan.lian

On Fri, Sep 22, 2017 at 03:25:21PM +0800, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> The Freescale PCIe controller advertises the MSI/MSI-X capability
> in both RC and Endpoint mode, but in RC mode it doesn't support
> MSI/MSI-X by it self, it can only transfer MSI/MSI-X from downstream

s/it self,/itself;/

> devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  drivers/pci/quirks.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index a4d33619a7bb..c1063a420f0c 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
>  /* AMD Stoney platform GPU */
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
>  #endif /* CONFIG_PCI_ATS */
> +
> +/* Freescale PCIe doesn't support MSI in RC mode */
> +static void quirk_fsl_no_msi(struct pci_dev *pdev)
> +{
> +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> +		pdev->no_msi = 1;
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, quirk_fsl_no_msi);

This disables MSI for all Freescale root ports, past, present, and
future.  Is that really what you want?  This is a bug (the root port
shouldn't advertise MSI if it doesn't support it), and presumably it
might be fixed in some future device?

This needs an ack from Minghuan or Mingkai (based on MAINTAINERS).

Bjorn

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
@ 2017-10-11 19:37     ` Bjorn Helgaas
  0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2017-10-11 19:37 UTC (permalink / raw)
  To: Zhiqiang Hou
  Cc: roy.zang, linux-pci, linux-kernel, minghuan.lian,
	linux-arm-kernel, bhelgaas, mingkai.hu

On Fri, Sep 22, 2017 at 03:25:21PM +0800, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> The Freescale PCIe controller advertises the MSI/MSI-X capability
> in both RC and Endpoint mode, but in RC mode it doesn't support
> MSI/MSI-X by it self, it can only transfer MSI/MSI-X from downstream

s/it self,/itself;/

> devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  drivers/pci/quirks.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index a4d33619a7bb..c1063a420f0c 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
>  /* AMD Stoney platform GPU */
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
>  #endif /* CONFIG_PCI_ATS */
> +
> +/* Freescale PCIe doesn't support MSI in RC mode */
> +static void quirk_fsl_no_msi(struct pci_dev *pdev)
> +{
> +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> +		pdev->no_msi = 1;
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, quirk_fsl_no_msi);

This disables MSI for all Freescale root ports, past, present, and
future.  Is that really what you want?  This is a bug (the root port
shouldn't advertise MSI if it doesn't support it), and presumably it
might be fixed in some future device?

This needs an ack from Minghuan or Mingkai (based on MAINTAINERS).

Bjorn

_______________________________________________
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] 22+ messages in thread

* [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
@ 2017-10-11 19:37     ` Bjorn Helgaas
  0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2017-10-11 19:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 22, 2017 at 03:25:21PM +0800, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> The Freescale PCIe controller advertises the MSI/MSI-X capability
> in both RC and Endpoint mode, but in RC mode it doesn't support
> MSI/MSI-X by it self, it can only transfer MSI/MSI-X from downstream

s/it self,/itself;/

> devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  drivers/pci/quirks.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index a4d33619a7bb..c1063a420f0c 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
>  /* AMD Stoney platform GPU */
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
>  #endif /* CONFIG_PCI_ATS */
> +
> +/* Freescale PCIe doesn't support MSI in RC mode */
> +static void quirk_fsl_no_msi(struct pci_dev *pdev)
> +{
> +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> +		pdev->no_msi = 1;
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, quirk_fsl_no_msi);

This disables MSI for all Freescale root ports, past, present, and
future.  Is that really what you want?  This is a bug (the root port
shouldn't advertise MSI if it doesn't support it), and presumably it
might be fixed in some future device?

This needs an ack from Minghuan or Mingkai (based on MAINTAINERS).

Bjorn

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] pci/layerscape: change the default error response behavior
  2017-09-22  7:25   ` Zhiqiang Hou
  (?)
@ 2017-10-11 19:41     ` Bjorn Helgaas
  -1 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2017-10-11 19:41 UTC (permalink / raw)
  To: Zhiqiang Hou
  Cc: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, roy.zang,
	mingkai.hu, minghuan.lian

On Fri, Sep 22, 2017 at 03:25:22PM +0800, Zhiqiang Hou wrote:
> From: Minghuan Lian <Minghuan.Lian@nxp.com>
> 
> 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>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pci-layerscape.c b/drivers/pci/dwc/pci-layerscape.c
> index 3b01e309a55e..a647090c140e 100644
> --- a/drivers/pci/dwc/pci-layerscape.c
> +++ b/drivers/pci/dwc/pci-layerscape.c
> @@ -33,6 +33,8 @@
>  
>  /* PEX Internal Configuration Registers */
>  #define PCIE_STRFMR1		0x71c /* Symbol Timer & Filter Mask Register1 */
> +#define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
> +#define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
>  
>  #define PCIE_IATU_NUM		6
>  
> @@ -54,6 +56,19 @@ struct ls_pcie {
>  
>  #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
>  
> +static int err_response_flag = 1;
> +
> +static int __init ls_pcie_param(char *p)
> +{
> +	if (p && strncmp(p, "no-err-response", 15) == 0)
> +		err_response_flag = 0;
> +	else
> +		err_response_flag = 1;
> +
> +	return 0;
> +}
> +early_param("ls_pcie", ls_pcie_param);

What's the point of this parameter?  If it's for debugging, it's not
clear that we need it upstream.  If it's for debugging and we *do*
need it upstream, there should be some sort of comment to that effect.

I assume you never expect an end user to need this parameter.

>  static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
>  {
>  	struct dw_pcie *pci = pcie->pci;
> @@ -124,6 +139,14 @@ static int ls_pcie_link_up(struct dw_pcie *pci)
>  	return 1;
>  }
>  
> +/* Forward error response of outbound non-posted requests */
> +static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
> +{
> +	struct dw_pcie *pci = pcie->pci;
> +
> +	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
> +}
> +
>  static int ls_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -135,6 +158,8 @@ static int ls_pcie_host_init(struct pcie_port *pp)
>  	 * dw_pcie_setup_rc() will reconfigure the outbound windows.
>  	 */
>  	ls_pcie_disable_outbound_atus(pcie);
> +	if (err_response_flag)
> +		ls_pcie_fix_error_response(pcie);
>  
>  	dw_pcie_dbi_ro_wr_en(pci);
>  	ls_pcie_clear_multifunction(pcie);
> -- 
> 2.14.1
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] pci/layerscape: change the default error response behavior
@ 2017-10-11 19:41     ` Bjorn Helgaas
  0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2017-10-11 19:41 UTC (permalink / raw)
  To: Zhiqiang Hou
  Cc: roy.zang, linux-pci, linux-kernel, minghuan.lian,
	linux-arm-kernel, bhelgaas, mingkai.hu

On Fri, Sep 22, 2017 at 03:25:22PM +0800, Zhiqiang Hou wrote:
> From: Minghuan Lian <Minghuan.Lian@nxp.com>
> 
> 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>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pci-layerscape.c b/drivers/pci/dwc/pci-layerscape.c
> index 3b01e309a55e..a647090c140e 100644
> --- a/drivers/pci/dwc/pci-layerscape.c
> +++ b/drivers/pci/dwc/pci-layerscape.c
> @@ -33,6 +33,8 @@
>  
>  /* PEX Internal Configuration Registers */
>  #define PCIE_STRFMR1		0x71c /* Symbol Timer & Filter Mask Register1 */
> +#define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
> +#define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
>  
>  #define PCIE_IATU_NUM		6
>  
> @@ -54,6 +56,19 @@ struct ls_pcie {
>  
>  #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
>  
> +static int err_response_flag = 1;
> +
> +static int __init ls_pcie_param(char *p)
> +{
> +	if (p && strncmp(p, "no-err-response", 15) == 0)
> +		err_response_flag = 0;
> +	else
> +		err_response_flag = 1;
> +
> +	return 0;
> +}
> +early_param("ls_pcie", ls_pcie_param);

What's the point of this parameter?  If it's for debugging, it's not
clear that we need it upstream.  If it's for debugging and we *do*
need it upstream, there should be some sort of comment to that effect.

I assume you never expect an end user to need this parameter.

>  static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
>  {
>  	struct dw_pcie *pci = pcie->pci;
> @@ -124,6 +139,14 @@ static int ls_pcie_link_up(struct dw_pcie *pci)
>  	return 1;
>  }
>  
> +/* Forward error response of outbound non-posted requests */
> +static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
> +{
> +	struct dw_pcie *pci = pcie->pci;
> +
> +	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
> +}
> +
>  static int ls_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -135,6 +158,8 @@ static int ls_pcie_host_init(struct pcie_port *pp)
>  	 * dw_pcie_setup_rc() will reconfigure the outbound windows.
>  	 */
>  	ls_pcie_disable_outbound_atus(pcie);
> +	if (err_response_flag)
> +		ls_pcie_fix_error_response(pcie);
>  
>  	dw_pcie_dbi_ro_wr_en(pci);
>  	ls_pcie_clear_multifunction(pcie);
> -- 
> 2.14.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] 22+ messages in thread

* [PATCH 2/2] pci/layerscape: change the default error response behavior
@ 2017-10-11 19:41     ` Bjorn Helgaas
  0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2017-10-11 19:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 22, 2017 at 03:25:22PM +0800, Zhiqiang Hou wrote:
> From: Minghuan Lian <Minghuan.Lian@nxp.com>
> 
> 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>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pci-layerscape.c b/drivers/pci/dwc/pci-layerscape.c
> index 3b01e309a55e..a647090c140e 100644
> --- a/drivers/pci/dwc/pci-layerscape.c
> +++ b/drivers/pci/dwc/pci-layerscape.c
> @@ -33,6 +33,8 @@
>  
>  /* PEX Internal Configuration Registers */
>  #define PCIE_STRFMR1		0x71c /* Symbol Timer & Filter Mask Register1 */
> +#define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
> +#define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
>  
>  #define PCIE_IATU_NUM		6
>  
> @@ -54,6 +56,19 @@ struct ls_pcie {
>  
>  #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
>  
> +static int err_response_flag = 1;
> +
> +static int __init ls_pcie_param(char *p)
> +{
> +	if (p && strncmp(p, "no-err-response", 15) == 0)
> +		err_response_flag = 0;
> +	else
> +		err_response_flag = 1;
> +
> +	return 0;
> +}
> +early_param("ls_pcie", ls_pcie_param);

What's the point of this parameter?  If it's for debugging, it's not
clear that we need it upstream.  If it's for debugging and we *do*
need it upstream, there should be some sort of comment to that effect.

I assume you never expect an end user to need this parameter.

>  static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
>  {
>  	struct dw_pcie *pci = pcie->pci;
> @@ -124,6 +139,14 @@ static int ls_pcie_link_up(struct dw_pcie *pci)
>  	return 1;
>  }
>  
> +/* Forward error response of outbound non-posted requests */
> +static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
> +{
> +	struct dw_pcie *pci = pcie->pci;
> +
> +	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
> +}
> +
>  static int ls_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -135,6 +158,8 @@ static int ls_pcie_host_init(struct pcie_port *pp)
>  	 * dw_pcie_setup_rc() will reconfigure the outbound windows.
>  	 */
>  	ls_pcie_disable_outbound_atus(pcie);
> +	if (err_response_flag)
> +		ls_pcie_fix_error_response(pcie);
>  
>  	dw_pcie_dbi_ro_wr_en(pci);
>  	ls_pcie_clear_multifunction(pcie);
> -- 
> 2.14.1
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
  2017-10-11 19:37     ` Bjorn Helgaas
  (?)
@ 2017-10-12  3:01       ` M.h. Lian
  -1 siblings, 0 replies; 22+ messages in thread
From: M.h. Lian @ 2017-10-12  3:01 UTC (permalink / raw)
  To: Bjorn Helgaas, Z.q. Hou
  Cc: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, Roy Zang,
	Mingkai Hu

Hi Bjorn,

Thanks for your review.
Yes. All the freescale's PCIe controllers do not support to generate MSI interrupt.
The PCIe controllers developed for the next generation SoC do not support it either.

Acked-by: Minghuan Lian <minghuan.Lian@nxp.com>

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: Thursday, October 12, 2017 3:38 AM
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> pci@vger.kernel.org; bhelgaas@google.com; Roy Zang <roy.zang@nxp.com>;
> Mingkai Hu <mingkai.hu@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
> 
> On Fri, Sep 22, 2017 at 03:25:21PM +0800, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The Freescale PCIe controller advertises the MSI/MSI-X capability in
> > both RC and Endpoint mode, but in RC mode it doesn't support MSI/MSI-X
> > by it self, it can only transfer MSI/MSI-X from downstream
> 
> s/it self,/itself;/
> 
> > devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/quirks.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > a4d33619a7bb..c1063a420f0c 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
> >  /* AMD Stoney platform GPU */
> >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
> > #endif /* CONFIG_PCI_ATS */
> > +
> > +/* Freescale PCIe doesn't support MSI in RC mode */ static void
> > +quirk_fsl_no_msi(struct pci_dev *pdev) {
> > +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> > +		pdev->no_msi = 1;
> > +}
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> > +quirk_fsl_no_msi);
> 
> This disables MSI for all Freescale root ports, past, present, and future.  Is that
> really what you want?  This is a bug (the root port shouldn't advertise MSI if it
> doesn't support it), and presumably it might be fixed in some future device?
> 
> This needs an ack from Minghuan or Mingkai (based on MAINTAINERS).
> 
> Bjorn

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
@ 2017-10-12  3:01       ` M.h. Lian
  0 siblings, 0 replies; 22+ messages in thread
From: M.h. Lian @ 2017-10-12  3:01 UTC (permalink / raw)
  To: Bjorn Helgaas, Z.q. Hou
  Cc: Roy Zang, linux-pci, linux-kernel, linux-arm-kernel, bhelgaas,
	Mingkai Hu

Hi Bjorn,

Thanks for your review.
Yes. All the freescale's PCIe controllers do not support to generate MSI interrupt.
The PCIe controllers developed for the next generation SoC do not support it either.

Acked-by: Minghuan Lian <minghuan.Lian@nxp.com>

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: Thursday, October 12, 2017 3:38 AM
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> pci@vger.kernel.org; bhelgaas@google.com; Roy Zang <roy.zang@nxp.com>;
> Mingkai Hu <mingkai.hu@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
> 
> On Fri, Sep 22, 2017 at 03:25:21PM +0800, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The Freescale PCIe controller advertises the MSI/MSI-X capability in
> > both RC and Endpoint mode, but in RC mode it doesn't support MSI/MSI-X
> > by it self, it can only transfer MSI/MSI-X from downstream
> 
> s/it self,/itself;/
> 
> > devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/quirks.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > a4d33619a7bb..c1063a420f0c 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
> >  /* AMD Stoney platform GPU */
> >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
> > #endif /* CONFIG_PCI_ATS */
> > +
> > +/* Freescale PCIe doesn't support MSI in RC mode */ static void
> > +quirk_fsl_no_msi(struct pci_dev *pdev) {
> > +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> > +		pdev->no_msi = 1;
> > +}
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> > +quirk_fsl_no_msi);
> 
> This disables MSI for all Freescale root ports, past, present, and future.  Is that
> really what you want?  This is a bug (the root port shouldn't advertise MSI if it
> doesn't support it), and presumably it might be fixed in some future device?
> 
> This needs an ack from Minghuan or Mingkai (based on MAINTAINERS).
> 
> Bjorn

_______________________________________________
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] 22+ messages in thread

* [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
@ 2017-10-12  3:01       ` M.h. Lian
  0 siblings, 0 replies; 22+ messages in thread
From: M.h. Lian @ 2017-10-12  3:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Bjorn,

Thanks for your review.
Yes. All the freescale's PCIe controllers do not support to generate MSI interrupt.
The PCIe controllers developed for the next generation SoC do not support it either.

Acked-by: Minghuan Lian <minghuan.Lian@nxp.com>

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> Sent: Thursday, October 12, 2017 3:38 AM
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
> pci at vger.kernel.org; bhelgaas at google.com; Roy Zang <roy.zang@nxp.com>;
> Mingkai Hu <mingkai.hu@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
> 
> On Fri, Sep 22, 2017 at 03:25:21PM +0800, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The Freescale PCIe controller advertises the MSI/MSI-X capability in
> > both RC and Endpoint mode, but in RC mode it doesn't support MSI/MSI-X
> > by it self, it can only transfer MSI/MSI-X from downstream
> 
> s/it self,/itself;/
> 
> > devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/quirks.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > a4d33619a7bb..c1063a420f0c 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
> >  /* AMD Stoney platform GPU */
> >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
> > #endif /* CONFIG_PCI_ATS */
> > +
> > +/* Freescale PCIe doesn't support MSI in RC mode */ static void
> > +quirk_fsl_no_msi(struct pci_dev *pdev) {
> > +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> > +		pdev->no_msi = 1;
> > +}
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> > +quirk_fsl_no_msi);
> 
> This disables MSI for all Freescale root ports, past, present, and future.  Is that
> really what you want?  This is a bug (the root port shouldn't advertise MSI if it
> doesn't support it), and presumably it might be fixed in some future device?
> 
> This needs an ack from Minghuan or Mingkai (based on MAINTAINERS).
> 
> Bjorn

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
  2017-10-11 19:37     ` Bjorn Helgaas
  (?)
@ 2017-10-12  3:17       ` Z.q. Hou
  -1 siblings, 0 replies; 22+ messages in thread
From: Z.q. Hou @ 2017-10-12  3:17 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, Roy Zang,
	Mingkai Hu, M.h. Lian

Hi Bjorn,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: 2017年10月12日 3:38
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-pci@vger.kernel.org; bhelgaas@google.com; Roy Zang
> <roy.zang@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; M.h. Lian
> <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
> 
> On Fri, Sep 22, 2017 at 03:25:21PM +0800, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The Freescale PCIe controller advertises the MSI/MSI-X capability in
> > both RC and Endpoint mode, but in RC mode it doesn't support MSI/MSI-X
> > by it self, it can only transfer MSI/MSI-X from downstream
> 
> s/it self,/itself;/

I'll fix this typo in next version.

> > devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/quirks.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > a4d33619a7bb..c1063a420f0c 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
> >  /* AMD Stoney platform GPU */
> >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
> > #endif /* CONFIG_PCI_ATS */
> > +
> > +/* Freescale PCIe doesn't support MSI in RC mode */ static void
> > +quirk_fsl_no_msi(struct pci_dev *pdev) {
> > +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> > +		pdev->no_msi = 1;
> > +}
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> > +quirk_fsl_no_msi);
> 
> This disables MSI for all Freescale root ports, past, present, and future.  Is
> that really what you want?  This is a bug (the root port shouldn't advertise
> MSI if it doesn't support it), and presumably it might be fixed in some future
> device?

For the past and present, there isn't Freescale root ports supporting MSI. If the future Freescale root port support MSI, I'll add a patch for it checking the PCI device ID to determine if apply the quirk.
And it should be ok for the root ports without this bug.

> 
> This needs an ack from Minghuan or Mingkai (based on MAINTAINERS).
> 

Thanks,
Zhiqiang

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
@ 2017-10-12  3:17       ` Z.q. Hou
  0 siblings, 0 replies; 22+ messages in thread
From: Z.q. Hou @ 2017-10-12  3:17 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, Roy Zang,
	Mingkai Hu, M.h. Lian

SGkgQmpvcm4sDQoNClRoYW5rcyBhIGxvdCBmb3IgeW91ciBjb21tZW50cyENCg0KPiAtLS0tLU9y
aWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBCam9ybiBIZWxnYWFzIFttYWlsdG86aGVsZ2Fh
c0BrZXJuZWwub3JnXQ0KPiBTZW50OiAyMDE3xOoxMNTCMTLI1SAzOjM4DQo+IFRvOiBaLnEuIEhv
dSA8emhpcWlhbmcuaG91QG54cC5jb20+DQo+IENjOiBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwu
b3JnOyBsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmc7DQo+IGxpbnV4LXBjaUB2
Z2VyLmtlcm5lbC5vcmc7IGJoZWxnYWFzQGdvb2dsZS5jb207IFJveSBaYW5nDQo+IDxyb3kuemFu
Z0BueHAuY29tPjsgTWluZ2thaSBIdSA8bWluZ2thaS5odUBueHAuY29tPjsgTS5oLiBMaWFuDQo+
IDxtaW5naHVhbi5saWFuQG54cC5jb20+DQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMS8yXSBQQ0k6
IERpc2FibGUgTVNJIGZvciBGcmVlc2NhbGUgUENJZSBSQyBtb2RlDQo+IA0KPiBPbiBGcmksIFNl
cCAyMiwgMjAxNyBhdCAwMzoyNToyMVBNICswODAwLCBaaGlxaWFuZyBIb3Ugd3JvdGU6DQo+ID4g
RnJvbTogSG91IFpoaXFpYW5nIDxaaGlxaWFuZy5Ib3VAbnhwLmNvbT4NCj4gPg0KPiA+IFRoZSBG
cmVlc2NhbGUgUENJZSBjb250cm9sbGVyIGFkdmVydGlzZXMgdGhlIE1TSS9NU0ktWCBjYXBhYmls
aXR5IGluDQo+ID4gYm90aCBSQyBhbmQgRW5kcG9pbnQgbW9kZSwgYnV0IGluIFJDIG1vZGUgaXQg
ZG9lc24ndCBzdXBwb3J0IE1TSS9NU0ktWA0KPiA+IGJ5IGl0IHNlbGYsIGl0IGNhbiBvbmx5IHRy
YW5zZmVyIE1TSS9NU0ktWCBmcm9tIGRvd25zdHJlYW0NCj4gDQo+IHMvaXQgc2VsZiwvaXRzZWxm
Oy8NCg0KSSdsbCBmaXggdGhpcyB0eXBvIGluIG5leHQgdmVyc2lvbi4NCg0KPiA+IGRldmljZXMu
IFNvIGFkZCB0aGlzIHF1aXJrIHRvIHByZXZlbnQgdXNlIG9mIE1TSS9NU0ktWCBpbiBSQyBtb2Rl
Lg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogSG91IFpoaXFpYW5nIDxaaGlxaWFuZy5Ib3VAbnhw
LmNvbT4NCj4gPiAtLS0NCj4gPiAgZHJpdmVycy9wY2kvcXVpcmtzLmMgfCA4ICsrKysrKysrDQo+
ID4gIDEgZmlsZSBjaGFuZ2VkLCA4IGluc2VydGlvbnMoKykNCj4gPg0KPiA+IGRpZmYgLS1naXQg
YS9kcml2ZXJzL3BjaS9xdWlya3MuYyBiL2RyaXZlcnMvcGNpL3F1aXJrcy5jIGluZGV4DQo+ID4g
YTRkMzM2MTlhN2JiLi5jMTA2M2E0MjBmMGMgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9wY2kv
cXVpcmtzLmMNCj4gPiArKysgYi9kcml2ZXJzL3BjaS9xdWlya3MuYw0KPiA+IEBAIC00Nzk5LDMg
KzQ3OTksMTEgQEAgc3RhdGljIHZvaWQgcXVpcmtfbm9fYXRzKHN0cnVjdCBwY2lfZGV2ICpwZGV2
KQ0KPiA+ICAvKiBBTUQgU3RvbmV5IHBsYXRmb3JtIEdQVSAqLw0KPiA+ICBERUNMQVJFX1BDSV9G
SVhVUF9GSU5BTChQQ0lfVkVORE9SX0lEX0FUSSwgMHg5OGU0LCBxdWlya19ub19hdHMpOw0KPiA+
ICNlbmRpZiAvKiBDT05GSUdfUENJX0FUUyAqLw0KPiA+ICsNCj4gPiArLyogRnJlZXNjYWxlIFBD
SWUgZG9lc24ndCBzdXBwb3J0IE1TSSBpbiBSQyBtb2RlICovIHN0YXRpYyB2b2lkDQo+ID4gK3F1
aXJrX2ZzbF9ub19tc2koc3RydWN0IHBjaV9kZXYgKnBkZXYpIHsNCj4gPiArCWlmIChwY2lfcGNp
ZV90eXBlKHBkZXYpID09IFBDSV9FWFBfVFlQRV9ST09UX1BPUlQpDQo+ID4gKwkJcGRldi0+bm9f
bXNpID0gMTsNCj4gPiArfQ0KPiA+ICtERUNMQVJFX1BDSV9GSVhVUF9GSU5BTChQQ0lfVkVORE9S
X0lEX0ZSRUVTQ0FMRSwgUENJX0FOWV9JRCwNCj4gPiArcXVpcmtfZnNsX25vX21zaSk7DQo+IA0K
PiBUaGlzIGRpc2FibGVzIE1TSSBmb3IgYWxsIEZyZWVzY2FsZSByb290IHBvcnRzLCBwYXN0LCBw
cmVzZW50LCBhbmQgZnV0dXJlLiAgSXMNCj4gdGhhdCByZWFsbHkgd2hhdCB5b3Ugd2FudD8gIFRo
aXMgaXMgYSBidWcgKHRoZSByb290IHBvcnQgc2hvdWxkbid0IGFkdmVydGlzZQ0KPiBNU0kgaWYg
aXQgZG9lc24ndCBzdXBwb3J0IGl0KSwgYW5kIHByZXN1bWFibHkgaXQgbWlnaHQgYmUgZml4ZWQg
aW4gc29tZSBmdXR1cmUNCj4gZGV2aWNlPw0KDQpGb3IgdGhlIHBhc3QgYW5kIHByZXNlbnQsIHRo
ZXJlIGlzbid0IEZyZWVzY2FsZSByb290IHBvcnRzIHN1cHBvcnRpbmcgTVNJLiBJZiB0aGUgZnV0
dXJlIEZyZWVzY2FsZSByb290IHBvcnQgc3VwcG9ydCBNU0ksIEknbGwgYWRkIGEgcGF0Y2ggZm9y
IGl0IGNoZWNraW5nIHRoZSBQQ0kgZGV2aWNlIElEIHRvIGRldGVybWluZSBpZiBhcHBseSB0aGUg
cXVpcmsuDQpBbmQgaXQgc2hvdWxkIGJlIG9rIGZvciB0aGUgcm9vdCBwb3J0cyB3aXRob3V0IHRo
aXMgYnVnLg0KDQo+IA0KPiBUaGlzIG5lZWRzIGFuIGFjayBmcm9tIE1pbmdodWFuIG9yIE1pbmdr
YWkgKGJhc2VkIG9uIE1BSU5UQUlORVJTKS4NCj4gDQoNClRoYW5rcywNClpoaXFpYW5nDQo=

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
@ 2017-10-12  3:17       ` Z.q. Hou
  0 siblings, 0 replies; 22+ messages in thread
From: Z.q. Hou @ 2017-10-12  3:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Bjorn,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> Sent: 2017?10?12? 3:38
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> linux-pci at vger.kernel.org; bhelgaas at google.com; Roy Zang
> <roy.zang@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; M.h. Lian
> <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode
> 
> On Fri, Sep 22, 2017 at 03:25:21PM +0800, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The Freescale PCIe controller advertises the MSI/MSI-X capability in
> > both RC and Endpoint mode, but in RC mode it doesn't support MSI/MSI-X
> > by it self, it can only transfer MSI/MSI-X from downstream
> 
> s/it self,/itself;/

I'll fix this typo in next version.

> > devices. So add this quirk to prevent use of MSI/MSI-X in RC mode.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/quirks.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > a4d33619a7bb..c1063a420f0c 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -4799,3 +4799,11 @@ static void quirk_no_ats(struct pci_dev *pdev)
> >  /* AMD Stoney platform GPU */
> >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_no_ats);
> > #endif /* CONFIG_PCI_ATS */
> > +
> > +/* Freescale PCIe doesn't support MSI in RC mode */ static void
> > +quirk_fsl_no_msi(struct pci_dev *pdev) {
> > +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> > +		pdev->no_msi = 1;
> > +}
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> > +quirk_fsl_no_msi);
> 
> This disables MSI for all Freescale root ports, past, present, and future.  Is
> that really what you want?  This is a bug (the root port shouldn't advertise
> MSI if it doesn't support it), and presumably it might be fixed in some future
> device?

For the past and present, there isn't Freescale root ports supporting MSI. If the future Freescale root port support MSI, I'll add a patch for it checking the PCI device ID to determine if apply the quirk.
And it should be ok for the root ports without this bug.

> 
> This needs an ack from Minghuan or Mingkai (based on MAINTAINERS).
> 

Thanks,
Zhiqiang

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 2/2] pci/layerscape: change the default error response behavior
  2017-10-11 19:41     ` Bjorn Helgaas
  (?)
@ 2017-10-12  3:33       ` Z.q. Hou
  -1 siblings, 0 replies; 22+ messages in thread
From: Z.q. Hou @ 2017-10-12  3:33 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, linux-arm-kernel, linux-pci, bhelgaas, Roy Zang,
	Mingkai Hu, M.h. Lian

Hi Bjorn,

Thanks a lot for your review!

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: 2017年10月12日 3:41
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-pci@vger.kernel.org; bhelgaas@google.com; Roy Zang
> <roy.zang@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; M.h. Lian
> <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 2/2] pci/layerscape: change the default error response
> behavior
> 
> On Fri, Sep 22, 2017 at 03:25:22PM +0800, Zhiqiang Hou wrote:
> > From: Minghuan Lian <Minghuan.Lian@nxp.com>
> >
> > 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>
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/drivers/pci/dwc/pci-layerscape.c
> > b/drivers/pci/dwc/pci-layerscape.c
> > index 3b01e309a55e..a647090c140e 100644
> > --- a/drivers/pci/dwc/pci-layerscape.c
> > +++ b/drivers/pci/dwc/pci-layerscape.c
> > @@ -33,6 +33,8 @@
> >
> >  /* PEX Internal Configuration Registers */
> >  #define PCIE_STRFMR1		0x71c /* Symbol Timer & Filter Mask
> Register1 */
> > +#define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response
> Register */
> > +#define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted
> request */
> >
> >  #define PCIE_IATU_NUM		6
> >
> > @@ -54,6 +56,19 @@ struct ls_pcie {
> >
> >  #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
> >
> > +static int err_response_flag = 1;
> > +
> > +static int __init ls_pcie_param(char *p) {
> > +	if (p && strncmp(p, "no-err-response", 15) == 0)
> > +		err_response_flag = 0;
> > +	else
> > +		err_response_flag = 1;
> > +
> > +	return 0;
> > +}
> > +early_param("ls_pcie", ls_pcie_param);
> 
> What's the point of this parameter?  If it's for debugging, it's not clear that
> we need it upstream.  If it's for debugging and we *do* need it upstream,
> there should be some sort of comment to that effect.
> 
> I assume you never expect an end user to need this parameter.

It is for debugging, will drop this parameter next version.

> 
> >  static bool ls_pcie_is_bridge(struct ls_pcie *pcie)  {
> >  	struct dw_pcie *pci = pcie->pci;
> > @@ -124,6 +139,14 @@ static int ls_pcie_link_up(struct dw_pcie *pci)
> >  	return 1;
> >  }
> >
> > +/* Forward error response of outbound non-posted requests */ static
> > +void ls_pcie_fix_error_response(struct ls_pcie *pcie) {
> > +	struct dw_pcie *pci = pcie->pci;
> > +
> > +	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR); }
> > +
> >  static int ls_pcie_host_init(struct pcie_port *pp)  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -135,6 +158,8 @@
> > static int ls_pcie_host_init(struct pcie_port *pp)
> >  	 * dw_pcie_setup_rc() will reconfigure the outbound windows.
> >  	 */
> >  	ls_pcie_disable_outbound_atus(pcie);
> > +	if (err_response_flag)
> > +		ls_pcie_fix_error_response(pcie);
> >
> >  	dw_pcie_dbi_ro_wr_en(pci);
> >  	ls_pcie_clear_multifunction(pcie);
> > --
> > 2.14.1
> >

Thanks,
Zhiqiang

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 2/2] pci/layerscape: change the default error response behavior
@ 2017-10-12  3:33       ` Z.q. Hou
  0 siblings, 0 replies; 22+ messages in thread
From: Z.q. Hou @ 2017-10-12  3:33 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Roy Zang, linux-pci, linux-kernel, M.h. Lian, linux-arm-kernel,
	bhelgaas, Mingkai Hu

SGkgQmpvcm4sDQoNClRoYW5rcyBhIGxvdCBmb3IgeW91ciByZXZpZXchDQoNCj4gLS0tLS1Pcmln
aW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmpvcm4gSGVsZ2FhcyBbbWFpbHRvOmhlbGdhYXNA
a2VybmVsLm9yZ10NCj4gU2VudDogMjAxN8TqMTDUwjEyyNUgMzo0MQ0KPiBUbzogWi5xLiBIb3Ug
PHpoaXFpYW5nLmhvdUBueHAuY29tPg0KPiBDYzogbGludXgta2VybmVsQHZnZXIua2VybmVsLm9y
ZzsgbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnOw0KPiBsaW51eC1wY2lAdmdl
ci5rZXJuZWwub3JnOyBiaGVsZ2Fhc0Bnb29nbGUuY29tOyBSb3kgWmFuZw0KPiA8cm95LnphbmdA
bnhwLmNvbT47IE1pbmdrYWkgSHUgPG1pbmdrYWkuaHVAbnhwLmNvbT47IE0uaC4gTGlhbg0KPiA8
bWluZ2h1YW4ubGlhbkBueHAuY29tPg0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDIvMl0gcGNpL2xh
eWVyc2NhcGU6IGNoYW5nZSB0aGUgZGVmYXVsdCBlcnJvciByZXNwb25zZQ0KPiBiZWhhdmlvcg0K
PiANCj4gT24gRnJpLCBTZXAgMjIsIDIwMTcgYXQgMDM6MjU6MjJQTSArMDgwMCwgWmhpcWlhbmcg
SG91IHdyb3RlOg0KPiA+IEZyb206IE1pbmdodWFuIExpYW4gPE1pbmdodWFuLkxpYW5AbnhwLmNv
bT4NCj4gPg0KPiA+IEJ5IGRlZmF1bHQsIHdoZW4gdGhlIFBDSWUgY29udHJvbGxlciBleHBlcmll
bmNlcyBhbiBlcnJvbmVvdXMNCj4gPiBjb21wbGV0aW9uIGZyb20gYW4gZXh0ZXJuYWwgY29tcGxl
dGVyIGZvciBpdHMgb3V0Ym91bmQgbm9uLXBvc3RlZA0KPiA+IHJlcXVlc3QsIGl0IGFsd2F5cyBz
ZW5kcyBhbiBPS0FZIHJlc3BvbnNlIHRvIHRoZSBkZXZpY2UncyBpbnRlcm5hbCBBWEkNCj4gPiBz
bGF2ZSBzeXN0ZW0gaW50ZXJmYWNlLiBIb3dldmVyLCBzdWNoIGRlZmF1bHQgc3lzdGVtIGVycm9y
IHJlc3BvbnNlDQo+ID4gYmVoYXZpb3IgY2Fubm90IGJlIHVzZWQgZm9yIG90aGVyIHR5cGVzIG9m
IG91dGJvdW5kIG5vbi1wb3N0ZWQNCj4gPiByZXF1ZXN0cy4gRm9yIGV4YW1wbGUsIHRoZSBvdXRi
b3VuZCBtZW1vcnkgcmVhZCB0cmFuc2FjdGlvbiByZXF1aXJlcw0KPiA+IGFuIGFjdHVhbCBFUlJP
UiByZXNwb25zZSwgbGlrZSBVUiBjb21wbGV0aW9uIG9yIGNvbXBsZXRpb24gdGltZW91dC4NCj4g
PiBUaGUgcGF0Y2ggaXMgdG8gZml4IGl0IGJ5IGZvcndhcmRpbmcgdGhlIGVycm9yIHJlc3BvbnNl
IG9mIHRoZQ0KPiA+IG5vbi1wb3N0ZWQgcmVxdWVzdC4NCj4gPg0KPiA+IFNpZ25lZC1vZmYtYnk6
IE1pbmdodWFuIExpYW4gPE1pbmdodWFuLkxpYW5AbnhwLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5
OiBIb3UgWmhpcWlhbmcgPFpoaXFpYW5nLkhvdUBueHAuY29tPg0KPiA+IC0tLQ0KPiA+ICBkcml2
ZXJzL3BjaS9kd2MvcGNpLWxheWVyc2NhcGUuYyB8IDI1ICsrKysrKysrKysrKysrKysrKysrKysr
KysNCj4gPiAgMSBmaWxlIGNoYW5nZWQsIDI1IGluc2VydGlvbnMoKykNCj4gPg0KPiA+IGRpZmYg
LS1naXQgYS9kcml2ZXJzL3BjaS9kd2MvcGNpLWxheWVyc2NhcGUuYw0KPiA+IGIvZHJpdmVycy9w
Y2kvZHdjL3BjaS1sYXllcnNjYXBlLmMNCj4gPiBpbmRleCAzYjAxZTMwOWE1NWUuLmE2NDcwOTBj
MTQwZSAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL3BjaS9kd2MvcGNpLWxheWVyc2NhcGUuYw0K
PiA+ICsrKyBiL2RyaXZlcnMvcGNpL2R3Yy9wY2ktbGF5ZXJzY2FwZS5jDQo+ID4gQEAgLTMzLDYg
KzMzLDggQEANCj4gPg0KPiA+ICAvKiBQRVggSW50ZXJuYWwgQ29uZmlndXJhdGlvbiBSZWdpc3Rl
cnMgKi8NCj4gPiAgI2RlZmluZSBQQ0lFX1NUUkZNUjEJCTB4NzFjIC8qIFN5bWJvbCBUaW1lciAm
IEZpbHRlciBNYXNrDQo+IFJlZ2lzdGVyMSAqLw0KPiA+ICsjZGVmaW5lIFBDSUVfQUJTRVJSCQkw
eDhkMCAvKiBCcmlkZ2UgU2xhdmUgRXJyb3IgUmVzcG9uc2UNCj4gUmVnaXN0ZXIgKi8NCj4gPiAr
I2RlZmluZSBQQ0lFX0FCU0VSUl9TRVRUSU5HCTB4OTQwMSAvKiBGb3J3YXJkIGVycm9yIG9mIG5v
bi1wb3N0ZWQNCj4gcmVxdWVzdCAqLw0KPiA+DQo+ID4gICNkZWZpbmUgUENJRV9JQVRVX05VTQkJ
Ng0KPiA+DQo+ID4gQEAgLTU0LDYgKzU2LDE5IEBAIHN0cnVjdCBsc19wY2llIHsNCj4gPg0KPiA+
ICAjZGVmaW5lIHRvX2xzX3BjaWUoeCkJZGV2X2dldF9kcnZkYXRhKCh4KS0+ZGV2KQ0KPiA+DQo+
ID4gK3N0YXRpYyBpbnQgZXJyX3Jlc3BvbnNlX2ZsYWcgPSAxOw0KPiA+ICsNCj4gPiArc3RhdGlj
IGludCBfX2luaXQgbHNfcGNpZV9wYXJhbShjaGFyICpwKSB7DQo+ID4gKwlpZiAocCAmJiBzdHJu
Y21wKHAsICJuby1lcnItcmVzcG9uc2UiLCAxNSkgPT0gMCkNCj4gPiArCQllcnJfcmVzcG9uc2Vf
ZmxhZyA9IDA7DQo+ID4gKwllbHNlDQo+ID4gKwkJZXJyX3Jlc3BvbnNlX2ZsYWcgPSAxOw0KPiA+
ICsNCj4gPiArCXJldHVybiAwOw0KPiA+ICt9DQo+ID4gK2Vhcmx5X3BhcmFtKCJsc19wY2llIiwg
bHNfcGNpZV9wYXJhbSk7DQo+IA0KPiBXaGF0J3MgdGhlIHBvaW50IG9mIHRoaXMgcGFyYW1ldGVy
PyAgSWYgaXQncyBmb3IgZGVidWdnaW5nLCBpdCdzIG5vdCBjbGVhciB0aGF0DQo+IHdlIG5lZWQg
aXQgdXBzdHJlYW0uICBJZiBpdCdzIGZvciBkZWJ1Z2dpbmcgYW5kIHdlICpkbyogbmVlZCBpdCB1
cHN0cmVhbSwNCj4gdGhlcmUgc2hvdWxkIGJlIHNvbWUgc29ydCBvZiBjb21tZW50IHRvIHRoYXQg
ZWZmZWN0Lg0KPiANCj4gSSBhc3N1bWUgeW91IG5ldmVyIGV4cGVjdCBhbiBlbmQgdXNlciB0byBu
ZWVkIHRoaXMgcGFyYW1ldGVyLg0KDQpJdCBpcyBmb3IgZGVidWdnaW5nLCB3aWxsIGRyb3AgdGhp
cyBwYXJhbWV0ZXIgbmV4dCB2ZXJzaW9uLg0KDQo+IA0KPiA+ICBzdGF0aWMgYm9vbCBsc19wY2ll
X2lzX2JyaWRnZShzdHJ1Y3QgbHNfcGNpZSAqcGNpZSkgIHsNCj4gPiAgCXN0cnVjdCBkd19wY2ll
ICpwY2kgPSBwY2llLT5wY2k7DQo+ID4gQEAgLTEyNCw2ICsxMzksMTQgQEAgc3RhdGljIGludCBs
c19wY2llX2xpbmtfdXAoc3RydWN0IGR3X3BjaWUgKnBjaSkNCj4gPiAgCXJldHVybiAxOw0KPiA+
ICB9DQo+ID4NCj4gPiArLyogRm9yd2FyZCBlcnJvciByZXNwb25zZSBvZiBvdXRib3VuZCBub24t
cG9zdGVkIHJlcXVlc3RzICovIHN0YXRpYw0KPiA+ICt2b2lkIGxzX3BjaWVfZml4X2Vycm9yX3Jl
c3BvbnNlKHN0cnVjdCBsc19wY2llICpwY2llKSB7DQo+ID4gKwlzdHJ1Y3QgZHdfcGNpZSAqcGNp
ID0gcGNpZS0+cGNpOw0KPiA+ICsNCj4gPiArCWlvd3JpdGUzMihQQ0lFX0FCU0VSUl9TRVRUSU5H
LCBwY2ktPmRiaV9iYXNlICsgUENJRV9BQlNFUlIpOyB9DQo+ID4gKw0KPiA+ICBzdGF0aWMgaW50
IGxzX3BjaWVfaG9zdF9pbml0KHN0cnVjdCBwY2llX3BvcnQgKnBwKSAgew0KPiA+ICAJc3RydWN0
IGR3X3BjaWUgKnBjaSA9IHRvX2R3X3BjaWVfZnJvbV9wcChwcCk7IEBAIC0xMzUsNiArMTU4LDgg
QEANCj4gPiBzdGF0aWMgaW50IGxzX3BjaWVfaG9zdF9pbml0KHN0cnVjdCBwY2llX3BvcnQgKnBw
KQ0KPiA+ICAJICogZHdfcGNpZV9zZXR1cF9yYygpIHdpbGwgcmVjb25maWd1cmUgdGhlIG91dGJv
dW5kIHdpbmRvd3MuDQo+ID4gIAkgKi8NCj4gPiAgCWxzX3BjaWVfZGlzYWJsZV9vdXRib3VuZF9h
dHVzKHBjaWUpOw0KPiA+ICsJaWYgKGVycl9yZXNwb25zZV9mbGFnKQ0KPiA+ICsJCWxzX3BjaWVf
Zml4X2Vycm9yX3Jlc3BvbnNlKHBjaWUpOw0KPiA+DQo+ID4gIAlkd19wY2llX2RiaV9yb193cl9l
bihwY2kpOw0KPiA+ICAJbHNfcGNpZV9jbGVhcl9tdWx0aWZ1bmN0aW9uKHBjaWUpOw0KPiA+IC0t
DQo+ID4gMi4xNC4xDQo+ID4NCg0KVGhhbmtzLA0KWmhpcWlhbmcNCl9fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LWFybS1rZXJuZWwgbWFpbGluZyBs
aXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5m
cmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFybS1rZXJuZWwK

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/2] pci/layerscape: change the default error response behavior
@ 2017-10-12  3:33       ` Z.q. Hou
  0 siblings, 0 replies; 22+ messages in thread
From: Z.q. Hou @ 2017-10-12  3:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Bjorn,

Thanks a lot for your review!

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> Sent: 2017?10?12? 3:41
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> linux-pci at vger.kernel.org; bhelgaas at google.com; Roy Zang
> <roy.zang@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; M.h. Lian
> <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 2/2] pci/layerscape: change the default error response
> behavior
> 
> On Fri, Sep 22, 2017 at 03:25:22PM +0800, Zhiqiang Hou wrote:
> > From: Minghuan Lian <Minghuan.Lian@nxp.com>
> >
> > 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>
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/dwc/pci-layerscape.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/drivers/pci/dwc/pci-layerscape.c
> > b/drivers/pci/dwc/pci-layerscape.c
> > index 3b01e309a55e..a647090c140e 100644
> > --- a/drivers/pci/dwc/pci-layerscape.c
> > +++ b/drivers/pci/dwc/pci-layerscape.c
> > @@ -33,6 +33,8 @@
> >
> >  /* PEX Internal Configuration Registers */
> >  #define PCIE_STRFMR1		0x71c /* Symbol Timer & Filter Mask
> Register1 */
> > +#define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response
> Register */
> > +#define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted
> request */
> >
> >  #define PCIE_IATU_NUM		6
> >
> > @@ -54,6 +56,19 @@ struct ls_pcie {
> >
> >  #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
> >
> > +static int err_response_flag = 1;
> > +
> > +static int __init ls_pcie_param(char *p) {
> > +	if (p && strncmp(p, "no-err-response", 15) == 0)
> > +		err_response_flag = 0;
> > +	else
> > +		err_response_flag = 1;
> > +
> > +	return 0;
> > +}
> > +early_param("ls_pcie", ls_pcie_param);
> 
> What's the point of this parameter?  If it's for debugging, it's not clear that
> we need it upstream.  If it's for debugging and we *do* need it upstream,
> there should be some sort of comment to that effect.
> 
> I assume you never expect an end user to need this parameter.

It is for debugging, will drop this parameter next version.

> 
> >  static bool ls_pcie_is_bridge(struct ls_pcie *pcie)  {
> >  	struct dw_pcie *pci = pcie->pci;
> > @@ -124,6 +139,14 @@ static int ls_pcie_link_up(struct dw_pcie *pci)
> >  	return 1;
> >  }
> >
> > +/* Forward error response of outbound non-posted requests */ static
> > +void ls_pcie_fix_error_response(struct ls_pcie *pcie) {
> > +	struct dw_pcie *pci = pcie->pci;
> > +
> > +	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR); }
> > +
> >  static int ls_pcie_host_init(struct pcie_port *pp)  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -135,6 +158,8 @@
> > static int ls_pcie_host_init(struct pcie_port *pp)
> >  	 * dw_pcie_setup_rc() will reconfigure the outbound windows.
> >  	 */
> >  	ls_pcie_disable_outbound_atus(pcie);
> > +	if (err_response_flag)
> > +		ls_pcie_fix_error_response(pcie);
> >
> >  	dw_pcie_dbi_ro_wr_en(pci);
> >  	ls_pcie_clear_multifunction(pcie);
> > --
> > 2.14.1
> >

Thanks,
Zhiqiang

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2017-10-12  3:33 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-22  7:25 [PATCH 0/2] PCI: layerscape: add fixes for layerscape-pcie errata Zhiqiang Hou
2017-09-22  7:25 ` Zhiqiang Hou
2017-09-22  7:25 ` Zhiqiang Hou
2017-09-22  7:25 ` [PATCH 1/2] PCI: Disable MSI for Freescale PCIe RC mode Zhiqiang Hou
2017-09-22  7:25   ` Zhiqiang Hou
2017-10-11 19:37   ` Bjorn Helgaas
2017-10-11 19:37     ` Bjorn Helgaas
2017-10-11 19:37     ` Bjorn Helgaas
2017-10-12  3:01     ` M.h. Lian
2017-10-12  3:01       ` M.h. Lian
2017-10-12  3:01       ` M.h. Lian
2017-10-12  3:17     ` Z.q. Hou
2017-10-12  3:17       ` Z.q. Hou
2017-10-12  3:17       ` Z.q. Hou
2017-09-22  7:25 ` [PATCH 2/2] pci/layerscape: change the default error response behavior Zhiqiang Hou
2017-09-22  7:25   ` Zhiqiang Hou
2017-10-11 19:41   ` Bjorn Helgaas
2017-10-11 19:41     ` Bjorn Helgaas
2017-10-11 19:41     ` Bjorn Helgaas
2017-10-12  3:33     ` Z.q. Hou
2017-10-12  3:33       ` Z.q. Hou
2017-10-12  3:33       ` Z.q. Hou

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.