Re: [RESEND PATCH v14 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe controller errors

2020-09-03 Thread Andy Shevchenko 
On Thu, Sep 3, 2020 at 1:43 PM Shiju Jose  wrote:
> >From: Andy Shevchenko [mailto:andriy.shevche...@linux.intel.com]
> >Sent: 01 September 2020 09:26
> >On Mon, Aug 31, 2020 at 10:26:06PM +0100, Shiju Jose wrote:
> >> From: Yicong Yang 

...

> >> +config PCIE_HISI_ERR
> >> +depends on ACPI_APEI_GHES && (ARM64 || COMPILE_TEST)
> >
> >> +depends on ACPI
> >
> >Isn't this implied by
> >   drivers/acpi/Kconfig:45:if ACPI
> >?
>
> This can be removed as  ACPI_APEI_GHES depends on ACPI.
>
> Do you have any other comments on this patch?

Nope. The rest is fine to me.

> >> +bool "HiSilicon HIP PCIe controller error handling driver"
> >> +help
> >> +  Say Y here if you want error handling support
> >> +  for the PCIe controller's errors on HiSilicon HIP SoCs

-- 
With Best Regards,
Andy Shevchenko

RE: [RESEND PATCH v14 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe controller errors

2020-09-03 Thread Shiju Jose 
Hi Andy,

>-Original Message-
>From: Andy Shevchenko [mailto:andriy.shevche...@linux.intel.com]
>Sent: 01 September 2020 09:26
>To: Shiju Jose 
>Cc: linux-a...@vger.kernel.org; linux-...@vger.kernel.org; linux-
>ker...@vger.kernel.org; r...@rjwysocki.net; helg...@kernel.org;
>b...@alien8.de; james.mo...@arm.com; lorenzo.pieral...@arm.com;
>r...@kernel.org; l...@kernel.org; tony.l...@intel.com;
>dan.carpen...@oracle.com; yangyicong ;
>Jonathan Cameron ; tanxiaofei
>; Linuxarm ; Bjorn
>Helgaas 
>Subject: Re: [RESEND PATCH v14 2/2] PCI: hip: Add handling of HiSilicon HIP
>PCIe controller errors
>
>On Mon, Aug 31, 2020 at 10:26:06PM +0100, Shiju Jose wrote:
>> From: Yicong Yang 
>>
>> The HiSilicon HIP PCIe controller is capable of handling errors on
>> root port and performing port reset separately at each root port.
>>
>> Add error handling driver for HIP PCIe controller to log and report
>> recoverable errors. Perform root port reset and restore link status
>> after the recovery.
>>
>> Following are some of the PCIe controller's recoverable errors 1.
>> completion transmission timeout error.
>> 2. CRS retry counter over the threshold error.
>> 3. ECC 2 bit errors
>> 4. AXI bresponse/rresponse errors etc.
>>
>> The driver placed in the drivers/pci/controller/ because the HIP PCIe
>> controller does not use DWC IP.
>>
>> Signed-off-by: Yicong Yang 
>> Signed-off-by: Shiju Jose 
>> Acked-by: Bjorn Helgaas 
>> --
>> drivers/pci/controller/Kconfig   |   8 +
>> drivers/pci/controller/Makefile  |   1 +
>> drivers/pci/controller/pcie-hisi-error.c | 336
>> +++
>> 3 files changed, 345 insertions(+)
>> create mode 100644 drivers/pci/controller/pcie-hisi-error.c
>> ---
>>  drivers/pci/controller/Kconfig   |   8 +
>>  drivers/pci/controller/Makefile  |   1 +
>>  drivers/pci/controller/pcie-hisi-error.c | 327
>> +++
>>  3 files changed, 336 insertions(+)
>>  create mode 100644 drivers/pci/controller/pcie-hisi-error.c
>>
>> diff --git a/drivers/pci/controller/Kconfig
>> b/drivers/pci/controller/Kconfig index f18c3725ef80..b1b8a8805dd8
>> 100644
>> --- a/drivers/pci/controller/Kconfig
>> +++ b/drivers/pci/controller/Kconfig
>> @@ -294,6 +294,14 @@ config PCI_LOONGSON
>>Say Y here if you want to enable PCI controller support on
>>Loongson systems.
>>
>> +config PCIE_HISI_ERR
>> +depends on ACPI_APEI_GHES && (ARM64 || COMPILE_TEST)
>
>> +depends on ACPI
>
>Isn't this implied by
>   drivers/acpi/Kconfig:45:if ACPI
>?

This can be removed as  ACPI_APEI_GHES depends on ACPI.

Do you have any other comments on this patch?

>
>> +bool "HiSilicon HIP PCIe controller error handling driver"
>> +help
>> +  Say Y here if you want error handling support
>> +  for the PCIe controller's errors on HiSilicon HIP SoCs
>> +
>>  source "drivers/pci/controller/dwc/Kconfig"
>>  source "drivers/pci/controller/mobiveil/Kconfig"
>>  source "drivers/pci/controller/cadence/Kconfig"
>> diff --git a/drivers/pci/controller/Makefile
>> b/drivers/pci/controller/Makefile index bcdbf49ab1e4..04c6edc285c5
>> 100644
>> --- a/drivers/pci/controller/Makefile
>> +++ b/drivers/pci/controller/Makefile
>> @@ -31,6 +31,7 @@ obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-
>tango.o
>>  obj-$(CONFIG_VMD) += vmd.o
>>  obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
>>  obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
>> +obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
>>  # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
>>  obj-y   += dwc/
>>  obj-y   += mobiveil/
>> diff --git a/drivers/pci/controller/pcie-hisi-error.c
>> b/drivers/pci/controller/pcie-hisi-error.c
>> new file mode 100644
>> index ..7959c9c8d2bc
>> --- /dev/null
>> +++ b/drivers/pci/controller/pcie-hisi-error.c
>> @@ -0,0 +1,327 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for handling the PCIe controller errors on
>> + * HiSilicon HIP SoCs.
>> + *
>> + * Copyright (c) 2020 HiSilicon Limited.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* HISI PCIe controller error definitions */
>> +#define HISI_PCIE_ERR_MISC_REGS 33

Re: [RESEND PATCH v14 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe controller errors

2020-09-01 Thread Andy Shevchenko 
On Mon, Aug 31, 2020 at 10:26:06PM +0100, Shiju Jose wrote:
> From: Yicong Yang 
> 
> The HiSilicon HIP PCIe controller is capable of handling errors
> on root port and performing port reset separately at each root port.
> 
> Add error handling driver for HIP PCIe controller to log
> and report recoverable errors. Perform root port reset and restore
> link status after the recovery.
> 
> Following are some of the PCIe controller's recoverable errors
> 1. completion transmission timeout error.
> 2. CRS retry counter over the threshold error.
> 3. ECC 2 bit errors
> 4. AXI bresponse/rresponse errors etc.
> 
> The driver placed in the drivers/pci/controller/ because the
> HIP PCIe controller does not use DWC IP.
> 
> Signed-off-by: Yicong Yang 
> Signed-off-by: Shiju Jose 
> Acked-by: Bjorn Helgaas 
> --
> drivers/pci/controller/Kconfig   |   8 +
> drivers/pci/controller/Makefile  |   1 +
> drivers/pci/controller/pcie-hisi-error.c | 336 +++
> 3 files changed, 345 insertions(+)
> create mode 100644 drivers/pci/controller/pcie-hisi-error.c
> ---
>  drivers/pci/controller/Kconfig   |   8 +
>  drivers/pci/controller/Makefile  |   1 +
>  drivers/pci/controller/pcie-hisi-error.c | 327 +++
>  3 files changed, 336 insertions(+)
>  create mode 100644 drivers/pci/controller/pcie-hisi-error.c
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index f18c3725ef80..b1b8a8805dd8 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -294,6 +294,14 @@ config PCI_LOONGSON
> Say Y here if you want to enable PCI controller support on
> Loongson systems.
>  
> +config PCIE_HISI_ERR
> + depends on ACPI_APEI_GHES && (ARM64 || COMPILE_TEST)

> + depends on ACPI

Isn't this implied by
drivers/acpi/Kconfig:45:if ACPI
?

> + bool "HiSilicon HIP PCIe controller error handling driver"
> + help
> +   Say Y here if you want error handling support
> +   for the PCIe controller's errors on HiSilicon HIP SoCs
> +
>  source "drivers/pci/controller/dwc/Kconfig"
>  source "drivers/pci/controller/mobiveil/Kconfig"
>  source "drivers/pci/controller/cadence/Kconfig"
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index bcdbf49ab1e4..04c6edc285c5 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
>  obj-$(CONFIG_VMD) += vmd.o
>  obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
>  obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
> +obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
>  # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
>  obj-y+= dwc/
>  obj-y+= mobiveil/
> diff --git a/drivers/pci/controller/pcie-hisi-error.c 
> b/drivers/pci/controller/pcie-hisi-error.c
> new file mode 100644
> index ..7959c9c8d2bc
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-hisi-error.c
> @@ -0,0 +1,327 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for handling the PCIe controller errors on
> + * HiSilicon HIP SoCs.
> + *
> + * Copyright (c) 2020 HiSilicon Limited.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* HISI PCIe controller error definitions */
> +#define HISI_PCIE_ERR_MISC_REGS  33
> +
> +#define HISI_PCIE_LOCAL_VALID_VERSIONBIT(0)
> +#define HISI_PCIE_LOCAL_VALID_SOC_ID BIT(1)
> +#define HISI_PCIE_LOCAL_VALID_SOCKET_ID  BIT(2)
> +#define HISI_PCIE_LOCAL_VALID_NIMBUS_ID  BIT(3)
> +#define HISI_PCIE_LOCAL_VALID_SUB_MODULE_ID  BIT(4)
> +#define HISI_PCIE_LOCAL_VALID_CORE_IDBIT(5)
> +#define HISI_PCIE_LOCAL_VALID_PORT_IDBIT(6)
> +#define HISI_PCIE_LOCAL_VALID_ERR_TYPE   BIT(7)
> +#define HISI_PCIE_LOCAL_VALID_ERR_SEVERITY   BIT(8)
> +#define HISI_PCIE_LOCAL_VALID_ERR_MISC   9
> +
> +static guid_t hisi_pcie_sec_guid =
> + GUID_INIT(0xB2889FC9, 0xE7D7, 0x4F9D,
> +   0xA8, 0x67, 0xAF, 0x42, 0xE9, 0x8B, 0xE7, 0x72);
> +
> +/*
> + * Firmware reports the socket port ID where the error occurred.  These
> + * macros convert that to the core ID and core port ID required by the
> + * ACPI reset method.
> + */
> +#define HISI_PCIE_PORT_ID(core, v)   (((v) >> 1) + ((core) << 3))
> +#define HISI_PCIE_CORE_ID(v) ((v) >> 3)
> +#define HISI_PCIE_CORE_PORT_ID(v)(((v) & 7) << 1)
> +
> +struct hisi_pcie_error_data {
> + u64 val_bits;
> + u8  version;
> + u8  soc_id;
> + u8  socket_id;
> + u8  nimbus_id;
> + u8  sub_module_id;
> + u8  core_id;
> + u8  port_id;
> + u8  err_severity;
> + u16 err_type;
> + u8  reserv[2];
> + u32

[RESEND PATCH v14 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe controller errors

2020-08-31 Thread Shiju Jose 
From: Yicong Yang 

The HiSilicon HIP PCIe controller is capable of handling errors
on root port and performing port reset separately at each root port.

Add error handling driver for HIP PCIe controller to log
and report recoverable errors. Perform root port reset and restore
link status after the recovery.

Following are some of the PCIe controller's recoverable errors
1. completion transmission timeout error.
2. CRS retry counter over the threshold error.
3. ECC 2 bit errors
4. AXI bresponse/rresponse errors etc.

The driver placed in the drivers/pci/controller/ because the
HIP PCIe controller does not use DWC IP.

Signed-off-by: Yicong Yang 
Signed-off-by: Shiju Jose 
Acked-by: Bjorn Helgaas 
--
drivers/pci/controller/Kconfig   |   8 +
drivers/pci/controller/Makefile  |   1 +
drivers/pci/controller/pcie-hisi-error.c | 336 +++
3 files changed, 345 insertions(+)
create mode 100644 drivers/pci/controller/pcie-hisi-error.c
---
 drivers/pci/controller/Kconfig   |   8 +
 drivers/pci/controller/Makefile  |   1 +
 drivers/pci/controller/pcie-hisi-error.c | 327 +++
 3 files changed, 336 insertions(+)
 create mode 100644 drivers/pci/controller/pcie-hisi-error.c

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index f18c3725ef80..b1b8a8805dd8 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -294,6 +294,14 @@ config PCI_LOONGSON
  Say Y here if you want to enable PCI controller support on
  Loongson systems.
 
+config PCIE_HISI_ERR
+   depends on ACPI_APEI_GHES && (ARM64 || COMPILE_TEST)
+   depends on ACPI
+   bool "HiSilicon HIP PCIe controller error handling driver"
+   help
+ Say Y here if you want error handling support
+ for the PCIe controller's errors on HiSilicon HIP SoCs
+
 source "drivers/pci/controller/dwc/Kconfig"
 source "drivers/pci/controller/mobiveil/Kconfig"
 source "drivers/pci/controller/cadence/Kconfig"
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index bcdbf49ab1e4..04c6edc285c5 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
 obj-$(CONFIG_VMD) += vmd.o
 obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
 obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
+obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
 # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
 obj-y  += dwc/
 obj-y  += mobiveil/
diff --git a/drivers/pci/controller/pcie-hisi-error.c 
b/drivers/pci/controller/pcie-hisi-error.c
new file mode 100644
index ..7959c9c8d2bc
--- /dev/null
+++ b/drivers/pci/controller/pcie-hisi-error.c
@@ -0,0 +1,327 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for handling the PCIe controller errors on
+ * HiSilicon HIP SoCs.
+ *
+ * Copyright (c) 2020 HiSilicon Limited.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* HISI PCIe controller error definitions */
+#define HISI_PCIE_ERR_MISC_REGS33
+
+#define HISI_PCIE_LOCAL_VALID_VERSION  BIT(0)
+#define HISI_PCIE_LOCAL_VALID_SOC_ID   BIT(1)
+#define HISI_PCIE_LOCAL_VALID_SOCKET_IDBIT(2)
+#define HISI_PCIE_LOCAL_VALID_NIMBUS_IDBIT(3)
+#define HISI_PCIE_LOCAL_VALID_SUB_MODULE_IDBIT(4)
+#define HISI_PCIE_LOCAL_VALID_CORE_ID  BIT(5)
+#define HISI_PCIE_LOCAL_VALID_PORT_ID  BIT(6)
+#define HISI_PCIE_LOCAL_VALID_ERR_TYPE BIT(7)
+#define HISI_PCIE_LOCAL_VALID_ERR_SEVERITY BIT(8)
+#define HISI_PCIE_LOCAL_VALID_ERR_MISC 9
+
+static guid_t hisi_pcie_sec_guid =
+   GUID_INIT(0xB2889FC9, 0xE7D7, 0x4F9D,
+ 0xA8, 0x67, 0xAF, 0x42, 0xE9, 0x8B, 0xE7, 0x72);
+
+/*
+ * Firmware reports the socket port ID where the error occurred.  These
+ * macros convert that to the core ID and core port ID required by the
+ * ACPI reset method.
+ */
+#define HISI_PCIE_PORT_ID(core, v)   (((v) >> 1) + ((core) << 3))
+#define HISI_PCIE_CORE_ID(v) ((v) >> 3)
+#define HISI_PCIE_CORE_PORT_ID(v)(((v) & 7) << 1)
+
+struct hisi_pcie_error_data {
+   u64 val_bits;
+   u8  version;
+   u8  soc_id;
+   u8  socket_id;
+   u8  nimbus_id;
+   u8  sub_module_id;
+   u8  core_id;
+   u8  port_id;
+   u8  err_severity;
+   u16 err_type;
+   u8  reserv[2];
+   u32 err_misc[HISI_PCIE_ERR_MISC_REGS];
+};
+
+struct hisi_pcie_error_private {
+   struct notifier_block   nb;
+   struct device *dev;
+};
+
+enum hisi_pcie_submodule_id {
+   HISI_PCIE_SUB_MODULE_ID_AP,
+   HISI_PCIE_SUB_MODULE_ID_TL,
+   HISI_PCIE_SUB_MODULE_ID_MAC,
+   HISI_PCIE_SUB_MODULE_ID_DL,
+   HISI_PCIE_SUB_MODULE_ID_SDI,
+};
+
+static const