[PATCH v4 0/2] Add support for RAS DES feature in PCIe DW

[PATCH v4 0/2] Add support for RAS DES feature in PCIe DW

[Shradha Todi]

DesignWare controller provides a vendor specific extended capability
called RASDES as an IP feature. This extended capability  provides
hardware information like:
 - Debug registers to know the state of the link or controller. 
 - Error injection mechanisms to inject various PCIe errors including
   sequence number, CRC
 - Statistical counters to know how many times a particular event
   occurred

However, in Linux we do not have any generic or custom support to be
able to use this feature in an efficient manner. This is the reason we
are proposing this framework. Debug and bring up time of high-speed IPs
are highly dependent on costlier hardware analyzers and this solution
will in some ways help to reduce the HW analyzer usage.

The debugfs entries can be used to get information about underlying
hardware and can be shared with user space. Separate debugfs entries has
been created to cater to all the DES hooks provided by the controller.
The debugfs entries interacts with the RASDES registers in the required
sequence and provides the meaningful data to the user. This eases the
effort to understand and use the register information for debugging.

v4:
    - Addressed comments from Manivannan, Bjorn and Jonathan
    - Addressed style related change requests from v3
    - Added Documentation under Documentation/ABI/testing and kdoc stype
      comments wherever required for better understanding
    - Enhanced error injection to include all possible error groups
    - Removed debugfs init call from common designware file and left it
      up to individual platform drivers to init/deinit as required.

v3: https://lore.kernel.org/all/20240625093813.112555-1-shradha.t@samsung.com/
    - v2 had suggestions about moving this framework to perf/EDAC instead of a
      controller specific debugfs but after discussions we decided to go ahead
      with the same. Rebased and posted v3 with minor style changes.

v2: https://lore.kernel.org/lkml/20231130115044.53512-1-shradha.t@samsung.com/
    - Addressed comments from Krzysztof Wilczyński, Bjorn Helgaas and
      posted v2 with a changed implementation for a better code design

v1: https://lore.kernel.org/all/20210518174618.42089-1-shradha.t@samsung.com/T/

Shradha Todi (2):
  PCI: dwc: Add support for vendor specific capability search
  PCI: dwc: Add debugfs based RASDES support in DWC

 Documentation/ABI/testing/debugfs-dwc-pcie    | 143 +++++
 drivers/pci/controller/dwc/Kconfig            |  11 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 .../controller/dwc/pcie-designware-debugfs.c  | 544 ++++++++++++++++++
 .../controller/dwc/pcie-designware-debugfs.h  |   0
 drivers/pci/controller/dwc/pcie-designware.c  |  16 +
 drivers/pci/controller/dwc/pcie-designware.h  |  18 +
 7 files changed, 733 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-dwc-pcie
 create mode 100644 drivers/pci/controller/dwc/pcie-designware-debugfs.c
 create mode 100644 drivers/pci/controller/dwc/pcie-designware-debugfs.h

-- 
2.17.1

[PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Shradha Todi]

Add vendor specific extended configuration space capability search API
using struct dw_pcie pointer for DW controllers.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 16 ++++++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 6d6cbc8b5b2c..41230c5e4a53 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
 	return 0;
 }
 
+u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
+{
+	u16 vsec = 0;
+	u32 header;
+
+	while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
+					PCI_EXT_CAP_ID_VNDR)) {
+		header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
+		if (PCI_VNDR_HEADER_ID(header) == vsec_cap)
+			return vsec;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dw_pcie_find_vsec_capability);
+
 u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap)
 {
 	return dw_pcie_find_next_ext_capability(pci, 0, cap);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 347ab74ac35a..98a057820bc7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -476,6 +476,7 @@ void dw_pcie_version_detect(struct dw_pcie *pci);
 
 u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
 u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap);
+u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap);
 
 int dw_pcie_read(void __iomem *addr, int size, u32 *val);
 int dw_pcie_write(void __iomem *addr, int size, u32 val);
-- 
2.17.1

[PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support in DWC

[Shradha Todi]

Add support to use the RASDES feature of DesignWare PCIe controller
using debugfs entries.

RASDES is a vendor specific extended PCIe capability which reads the
current hardware internal state of PCIe device. Following primary
features are provided to userspace via debugfs:
- Debug registers
- Error injection
- Statistical counters

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 Documentation/ABI/testing/debugfs-dwc-pcie    | 143 +++++
 drivers/pci/controller/dwc/Kconfig            |  11 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 .../controller/dwc/pcie-designware-debugfs.c  | 544 ++++++++++++++++++
 .../controller/dwc/pcie-designware-debugfs.h  |   0
 drivers/pci/controller/dwc/pcie-designware.h  |  17 +
 6 files changed, 716 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-dwc-pcie
 create mode 100644 drivers/pci/controller/dwc/pcie-designware-debugfs.c
 create mode 100644 drivers/pci/controller/dwc/pcie-designware-debugfs.h

diff --git a/Documentation/ABI/testing/debugfs-dwc-pcie b/Documentation/ABI/testing/debugfs-dwc-pcie
new file mode 100644
index 000000000000..7da73ac8d40c
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-dwc-pcie
@@ -0,0 +1,143 @@
+What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/lane_detect
+Date:		December 2024
+Contact:	Shradha Todi <shradha.t@samsung.com>
+Description:	(RW) Write the lane number to be checked for detection.	Read
+		will dump whether PHY indicates receiver detection on the
+		selected lane.
+
+What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/rx_valid
+Date:		December 2024
+Contact:	Shradha Todi <shradha.t@samsung.com>
+Description:	(RW) Write the lane number to be checked as valid or invalid. Read
+		will dump the status of PIPE RXVALID signal of the selected lane.
+
+What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_event_counters/<event>/counter_enable
+Date:		December 2024
+Contact:	Shradha Todi <shradha.t@samsung.com>
+Description:	rasdes_event_counters is the directory which can be used to collect
+		statistical data about the number of times a certain event has occurred
+		in the controller. The list of possible events are:
+
+		1) EBUF Overflow
+		2) EBUF Underrun
+		3) Decode Error
+		4) Running Disparity Error
+		5) SKP OS Parity Error
+		6) SYNC Header Error
+		7) Rx Valid De-assertion
+		8) CTL SKP OS Parity Error
+		9) 1st Retimer Parity Error
+		10) 2nd Retimer Parity Error
+		11) Margin CRC and Parity Error
+		12) Detect EI Infer
+		13) Receiver Error
+		14) RX Recovery Req
+		15) N_FTS Timeout
+		16) Framing Error
+		17) Deskew Error
+		18) Framing Error In L0
+		19) Deskew Uncompleted Error
+		20) Bad TLP
+		21) LCRC Error
+		22) Bad DLLP
+		23) Replay Number Rollover
+		24) Replay Timeout
+		25) Rx Nak DLLP
+		26) Tx Nak DLLP
+		27) Retry TLP
+		28) FC Timeout
+		29) Poisoned TLP
+		30) ECRC Error
+		31) Unsupported Request
+		32) Completer Abort
+		33) Completion Timeout
+		34) EBUF SKP Add
+		35) EBUF SKP Del
+
+		counter_enable is RW. Write 1 to enable the event counter and write 0 to
+		disable the event counter. Read will dump whether the counter is currently
+		enabled	or disabled.
+
+What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_event_counters/<event>/counter_value
+Date:		December 2024
+Contact:	Shradha Todi <shradha.t@samsung.com>
+Description:	(RO) Read will dump the current value of the event counter. To reset the counter,
+		we can disable and re-enable the counter.
+
+What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_event_counters/<event>/lane_select
+Date:		December 2024
+Contact:	Shradha Todi <shradha.t@samsung.com>
+Description:	(RW) Some lanes in the event list are lane specific events. These include
+		events 1) - 11) and 34) - 35).
+		Write lane number for which counter needs to be enabled/disabled/dumped.
+		Read will dump the current selected lane number.
+
+What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_err_inj/<error>
+Date:		December 2024
+Contact:	Shradha Todi <shradha.t@samsung.com>
+Description:	rasdes_err_inj is the directory which can be used to inject errors in the
+		system. The possible errors that can be injected are:
+
+		1) TLP LCRC error injection TX Path - tx_lcrc
+		2) 16b CRC error injection of ACK/NAK DLLP - b16_crc_dllp
+		3) 16b CRC error injection of Update-FC DLLP - b16_crc_upd_fc
+		4) TLP ECRC error injection TX Path - tx_ecrc
+		5) TLP's FCRC error injection TX Path - fcrc_tlp
+		6) Parity error of TSOS - parity_tsos
+		7) Parity error on SKPOS - parity_skpos
+		8) LCRC error injection RX Path - rx_lcrc
+		9) ECRC error injection RX Path - rx_ecrc
+		10) TLPs SEQ# error - tlp_err_seq
+		11) DLLPS ACK/NAK SEQ# error - ack_nak_dllp_seq
+		12) ACK/NAK DLLPs transmission block - ack_nak_dllp
+		13) UpdateFC DLLPs transmission block - upd_fc_dllp
+		14) Always transmission for NAK DLLP - nak_dllp
+		15) Invert SYNC header - inv_sync_hdr_sym
+		16) COM/PAD TS1 order set - com_pad_ts1
+		17) COM/PAD TS2 order set - com_pad_ts2
+		18) COM/FTS FTS order set - com_fts
+		19) COM/IDL E-idle order set - com_idl
+		20) END/EDB symbol - end_edb
+		21) STP/SDP symbol - stp_sdp
+		22) COM/SKP SKP order set - com_skp
+		23) Posted TLP Header credit value control - posted_tlp_hdr
+		24) Non-Posted TLP Header credit value control - non_post_tlp_hdr
+		25) Completion TLP Header credit value control - cmpl_tlp_hdr
+		26) Posted TLP Data credit value control - posted_tlp_data
+		27) Non-Posted TLP Data credit value control - non_post_tlp_data
+		28) Completion TLP Data credit value control - cmpl_tlp_data
+		29) Generates duplicate TLPs - duplicate_dllp
+		30) Generates Nullified TLPs - nullified_tlp
+
+		Each of the possible errors are WO files. Write to the file will prepare
+		controller to inject the respective error in the next transmission of data.
+		Parameter required to write will change in the following ways:
+
+		i) Errors 9) - 10) are sequence errors. The write command for these will be
+
+			echo <count> <diff> > /sys/kernel/debug/dwc_pcie_<dev>/rasdes_err_inj/<error>
+
+			<count>
+				Number of errors to be injected
+			<diff>
+				The difference to add or subtract from natural sequence number to
+				generate sequence error. Range (-4095 : 4095)
+
+		ii) Errors 23) - 28) are credit value error insertions. Write command:
+
+			echo <count> <diff> <vc> > /sys/kernel/debug/dwc_pcie_<dev>/rasdes_err_inj/<error>
+
+			<count>
+				Number of errors to be injected
+			<diff>
+				The difference to add or subtract from UpdateFC credit value.
+				Range (-4095 : 4095)
+			<vc>
+				Target VC number
+
+		iii) All other errors. Write command:
+
+			echo <count> > /sys/kernel/debug/dwc_pcie_<dev>/rasdes_err_inj/<error>
+
+			<count>
+				Number of errors to be injected
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index b6d6778b0698..9ab8d724fe0d 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -6,6 +6,17 @@ menu "DesignWare-based PCIe controllers"
 config PCIE_DW
 	bool
 
+config PCIE_DW_DEBUGFS
+	default y
+	depends on DEBUG_FS
+	depends on PCIE_DW_HOST || PCIE_DW_EP
+	bool "DWC PCIe debugfs entries"
+	help
+	  Enables debugfs entries for the DWC PCIe Controller.
+	  These entries make use of the RAS features in the DW
+	  controller to help in debug, error injection and statistical
+	  counters
+
 config PCIE_DW_HOST
 	bool
 	select PCIE_DW
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index a8308d9ea986..54565eedc52c 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_PCIE_DW) += pcie-designware.o
+obj-$(CONFIG_PCIE_DW_DEBUGFS) += pcie-designware-debugfs.o
 obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
 obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
 obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.c b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
new file mode 100644
index 000000000000..a93e29993f75
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
@@ -0,0 +1,544 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Synopsys DesignWare PCIe controller debugfs driver
+ *
+ * Copyright (C) 2024 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Author: Shradha Todi <shradha.t@samsung.com>
+ */
+
+#include <linux/debugfs.h>
+
+#include "pcie-designware.h"
+
+#define SD_STATUS_L1LANE_REG		0xb0
+#define PIPE_DETECT_LANE		BIT(17)
+#define PIPE_RXVALID			BIT(18)
+#define LANE_SELECT			GENMASK(3, 0)
+
+#define RAS_DES_EVENT_COUNTER_CTRL_REG	0x8
+#define EVENT_COUNTER_GROUP_SELECT	GENMASK(27, 24)
+#define EVENT_COUNTER_EVENT_SELECT	GENMASK(23, 16)
+#define EVENT_COUNTER_LANE_SELECT	GENMASK(11, 8)
+#define EVENT_COUNTER_STATUS		BIT(7)
+#define EVENT_COUNTER_ENABLE		GENMASK(4, 2)
+#define PER_EVENT_OFF			0x1
+#define PER_EVENT_ON			0x3
+
+#define RAS_DES_EVENT_COUNTER_DATA_REG	0xc
+
+#define ERR_INJ_ENABLE_REG		0x30
+#define ERR_INJ0_OFF			0x34
+#define EINJ_COUNT			GENMASK(7, 0)
+#define EINJ_TYPE_SHIFT			8
+#define EINJ0_TYPE			GENMASK(11, 8)
+#define EINJ1_TYPE			BIT(8)
+#define EINJ2_TYPE			GENMASK(9, 8)
+#define EINJ3_TYPE			GENMASK(10, 8)
+#define EINJ4_TYPE			GENMASK(10, 8)
+#define EINJ5_TYPE			BIT(8)
+#define EINJ_VC_NUM			GENMASK(14, 12)
+#define EINJ_VAL_DIFF			GENMASK(28, 16)
+
+#define DWC_DEBUGFS_BUF_MAX		128
+
+/**
+ * struct dwc_pcie_rasdes_info - Stores controller common information
+ * @ras_cap_offset: RAS DES vendor specific extended capability offset
+ * @reg_lock: Mutex used for RASDES shadow event registers
+ * @rasdes_dir: Top level debugfs directory entry
+ *
+ * Any parameter constant to all files of the debugfs hierarchy for a single controller
+ * will be stored in this struct. It is allocated and assigned to controller specific
+ * struct dw_pcie during initialization.
+ */
+struct dwc_pcie_rasdes_info {
+	u32 ras_cap_offset;
+	struct mutex reg_lock;
+	struct dentry *rasdes_dir;
+};
+
+/**
+ * struct dwc_pcie_rasdes_priv - Stores file specific private data information
+ * @pci: Reference to the dw_pcie structure
+ * @idx: Index to point to specific file related information in array of structs
+ *
+ * All debugfs files will have this struct as its private data.
+ */
+struct dwc_pcie_rasdes_priv {
+	struct dw_pcie *pci;
+	int idx;
+};
+
+/**
+ * struct dwc_pcie_event_counter - Store details about each event counter supported in DWC RASDES
+ * @name: Name of the error counter
+ * @group_no: Group number that the event belongs to. Value ranges from 0 - 4
+ * @event_no: Event number of the particular event. Value ranges from -
+ *		Group 0: 0 - 10
+ *		Group 1: 5 - 13
+ *		Group 2: 0 - 7
+ *		Group 3: 0 - 5
+ *		Group 4: 0 - 1
+ */
+struct dwc_pcie_event_counter {
+	const char *name;
+	u32 group_no;
+	u32 event_no;
+};
+
+static const struct dwc_pcie_event_counter event_list[] = {
+	{"ebuf_overflow", 0x0, 0x0},
+	{"ebuf_underrun", 0x0, 0x1},
+	{"decode_err", 0x0, 0x2},
+	{"running_disparity_err", 0x0, 0x3},
+	{"skp_os_parity_err", 0x0, 0x4},
+	{"sync_header_err", 0x0, 0x5},
+	{"rx_valid_deassertion", 0x0, 0x6},
+	{"ctl_skp_os_parity_err", 0x0, 0x7},
+	{"retimer_parity_err_1st", 0x0, 0x8},
+	{"retimer_parity_err_2nd", 0x0, 0x9},
+	{"margin_crc_parity_err", 0x0, 0xA},
+	{"detect_ei_infer", 0x1, 0x5},
+	{"receiver_err", 0x1, 0x6},
+	{"rx_recovery_req", 0x1, 0x7},
+	{"n_fts_timeout", 0x1, 0x8},
+	{"framing_err", 0x1, 0x9},
+	{"deskew_err", 0x1, 0xa},
+	{"framing_err_in_l0", 0x1, 0xc},
+	{"deskew_uncompleted_err", 0x1, 0xd},
+	{"bad_tlp", 0x2, 0x0},
+	{"lcrc_err", 0x2, 0x1},
+	{"bad_dllp", 0x2, 0x2},
+	{"replay_num_rollover", 0x2, 0x3},
+	{"replay_timeout", 0x2, 0x4},
+	{"rx_nak_dllp", 0x2, 0x5},
+	{"tx_nak_dllp", 0x2, 0x6},
+	{"retry_tlp", 0x2, 0x7},
+	{"fc_timeout", 0x3, 0x0},
+	{"poisoned_tlp", 0x3, 0x1},
+	{"ecrc_error", 0x3, 0x2},
+	{"unsupported_request", 0x3, 0x3},
+	{"completer_abort", 0x3, 0x4},
+	{"completion_timeout", 0x3, 0x5},
+	{"ebuf_skp_add", 0x4, 0x0},
+	{"ebuf_skp_del", 0x4, 0x1},
+};
+
+/**
+ * struct dwc_pcie_err_inj - Store details about each error injection supported by DWC RASDES
+ * @name: Name of the error that can be injected
+ * @err_inj_group: Group number to which the error belongs to. Value can range from 0 - 5
+ * @err_inj_type: Each group can have multiple types of error
+ */
+struct dwc_pcie_err_inj {
+	const char *name;
+	u32 err_inj_group;
+	u32 err_inj_type;
+};
+
+static const struct dwc_pcie_err_inj err_inj_list[] = {
+	{"tx_lcrc", 0x0, 0x0},
+	{"b16_crc_dllp", 0x0, 0x1},
+	{"b16_crc_upd_fc", 0x0, 0x2},
+	{"tx_ecrc", 0x0, 0x3},
+	{"fcrc_tlp", 0x0, 0x4},
+	{"parity_tsos", 0x0, 0x5},
+	{"parity_skpos", 0x0, 0x6},
+	{"rx_lcrc", 0x0, 0x8},
+	{"rx_ecrc", 0x0, 0xb},
+	{"tlp_err_seq", 0x1, 0x0},
+	{"ack_nak_dllp_seq", 0x1, 0x1},
+	{"ack_nak_dllp", 0x2, 0x0},
+	{"upd_fc_dllp", 0x2, 0x1},
+	{"nak_dllp", 0x2, 0x2},
+	{"inv_sync_hdr_sym", 0x3, 0x0},
+	{"com_pad_ts1", 0x3, 0x1},
+	{"com_pad_ts2", 0x3, 0x2},
+	{"com_fts", 0x3, 0x3},
+	{"com_idl", 0x3, 0x4},
+	{"end_edb", 0x3, 0x5},
+	{"stp_sdp", 0x3, 0x6},
+	{"com_skp", 0x3, 0x7},
+	{"posted_tlp_hdr", 0x4, 0x0},
+	{"non_post_tlp_hdr", 0x4, 0x1},
+	{"cmpl_tlp_hdr", 0x4, 0x2},
+	{"posted_tlp_data", 0x4, 0x4},
+	{"non_post_tlp_data", 0x4, 0x5},
+	{"cmpl_tlp_data", 0x4, 0x6},
+	{"duplicate_dllp", 0x5, 0x0},
+	{"nullified_tlp", 0x5, 0x1},
+};
+
+static const u32 err_inj_type_mask[] = {
+	EINJ0_TYPE,
+	EINJ1_TYPE,
+	EINJ2_TYPE,
+	EINJ3_TYPE,
+	EINJ4_TYPE,
+	EINJ5_TYPE,
+};
+
+static ssize_t lane_detect_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
+{
+	struct dw_pcie *pci = file->private_data;
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
+	ssize_t off = 0;
+	u32 val;
+
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
+	val = FIELD_GET(PIPE_DETECT_LANE, val);
+	if (val)
+		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane Detected\n");
+	else
+		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane Undetected\n");
+
+	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
+}
+
+static ssize_t lane_detect_write(struct file *file, const char __user *buf,
+				 size_t count, loff_t *ppos)
+{
+	struct dw_pcie *pci = file->private_data;
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+	u32 lane, val;
+
+	val = kstrtou32_from_user(buf, count, 0, &lane);
+	if (val)
+		return val;
+
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
+	val &= ~(LANE_SELECT);
+	val |= FIELD_PREP(LANE_SELECT, lane);
+	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG, val);
+
+	return count;
+}
+
+static ssize_t rx_valid_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
+{
+	struct dw_pcie *pci = file->private_data;
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
+	ssize_t off = 0;
+	u32 val;
+
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
+	val = FIELD_GET(PIPE_RXVALID, val);
+	if (val)
+		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "RX Valid\n");
+	else
+		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "RX Invalid\n");
+
+	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
+}
+
+static ssize_t rx_valid_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
+{
+	return lane_detect_write(file, buf, count, ppos);
+}
+
+static void set_event_number(struct dwc_pcie_rasdes_priv *pdata, struct dw_pcie *pci,
+			     struct dwc_pcie_rasdes_info *rinfo)
+{
+	u32 val;
+
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
+	val &= ~EVENT_COUNTER_ENABLE;
+	val &= ~(EVENT_COUNTER_GROUP_SELECT | EVENT_COUNTER_EVENT_SELECT);
+	val |= FIELD_PREP(EVENT_COUNTER_GROUP_SELECT, event_list[pdata->idx].group_no);
+	val |= FIELD_PREP(EVENT_COUNTER_EVENT_SELECT, event_list[pdata->idx].event_no);
+	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
+}
+
+static ssize_t counter_enable_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
+{
+	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
+	struct dw_pcie *pci = pdata->pci;
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
+	ssize_t off = 0;
+	u32 val;
+
+	mutex_lock(&rinfo->reg_lock);
+	set_event_number(pdata, pci, rinfo);
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
+	mutex_unlock(&rinfo->reg_lock);
+	val = FIELD_GET(EVENT_COUNTER_STATUS, val);
+	if (val)
+		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter Enabled\n");
+	else
+		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter Disabled\n");
+
+	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
+}
+
+static ssize_t counter_enable_write(struct file *file, const char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
+	struct dw_pcie *pci = pdata->pci;
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+	u32 val, enable;
+
+	val = kstrtou32_from_user(buf, count, 0, &enable);
+	if (val)
+		return val;
+
+	mutex_lock(&rinfo->reg_lock);
+	set_event_number(pdata, pci, rinfo);
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
+	if (enable)
+		val |= FIELD_PREP(EVENT_COUNTER_ENABLE, PER_EVENT_ON);
+	else
+		val |= FIELD_PREP(EVENT_COUNTER_ENABLE, PER_EVENT_OFF);
+
+	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
+	mutex_unlock(&rinfo->reg_lock);
+
+	return count;
+}
+
+static ssize_t counter_lane_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
+{
+	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
+	struct dw_pcie *pci = pdata->pci;
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
+	ssize_t off = 0;
+	u32 val;
+
+	mutex_lock(&rinfo->reg_lock);
+	set_event_number(pdata, pci, rinfo);
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
+	mutex_unlock(&rinfo->reg_lock);
+	val = FIELD_GET(EVENT_COUNTER_LANE_SELECT, val);
+	off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane: %d\n", val);
+
+	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
+}
+
+static ssize_t counter_lane_write(struct file *file, const char __user *buf,
+				  size_t count, loff_t *ppos)
+{
+	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
+	struct dw_pcie *pci = pdata->pci;
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+	u32 val, lane;
+
+	val = kstrtou32_from_user(buf, count, 0, &lane);
+	if (val)
+		return val;
+
+	mutex_lock(&rinfo->reg_lock);
+	set_event_number(pdata, pci, rinfo);
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
+	val &= ~(EVENT_COUNTER_LANE_SELECT);
+	val |= FIELD_PREP(EVENT_COUNTER_LANE_SELECT, lane);
+	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
+	mutex_unlock(&rinfo->reg_lock);
+
+	return count;
+}
+
+static ssize_t counter_value_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
+{
+	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
+	struct dw_pcie *pci = pdata->pci;
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
+	ssize_t off = 0;
+	u32 val;
+
+	mutex_lock(&rinfo->reg_lock);
+	set_event_number(pdata, pci, rinfo);
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_DATA_REG);
+	mutex_unlock(&rinfo->reg_lock);
+	off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter value: %d\n", val);
+
+	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
+}
+
+static ssize_t err_inj_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
+{
+	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
+	struct dw_pcie *pci = pdata->pci;
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+	u32 val, counter, vc_num, err_group, type_mask;
+	int val_diff = 0;
+	char *kern_buf;
+
+	err_group = err_inj_list[pdata->idx].err_inj_group;
+	type_mask = err_inj_type_mask[err_group];
+
+	kern_buf = memdup_user_nul(buf, count);
+	if (IS_ERR(kern_buf))
+		return PTR_ERR(kern_buf);
+
+	if (err_group == 4) {
+		val = sscanf(kern_buf, "%u %d %u", &counter, &val_diff, &vc_num);
+		if ((val != 3) || (val_diff < -4095 || val_diff > 4095)) {
+			kfree(kern_buf);
+			return -EINVAL;
+		}
+	} else if (err_group == 1) {
+		val = sscanf(kern_buf, "%u %d", &counter, &val_diff);
+		if ((val != 2) || (val_diff < -4095 || val_diff > 4095)) {
+			kfree(kern_buf);
+			return -EINVAL;
+		}
+	} else {
+		val = kstrtou32(kern_buf, 0, &counter);
+		if (val) {
+			kfree(kern_buf);
+			return val;
+		}
+	}
+
+	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + ERR_INJ0_OFF + (0x4 * err_group));
+	val &= ~(type_mask | EINJ_COUNT);
+	val |= ((err_inj_list[pdata->idx].err_inj_type << EINJ_TYPE_SHIFT) & type_mask);
+	val |= FIELD_PREP(EINJ_COUNT, counter);
+
+	if (err_group == 1 || err_group == 4) {
+		val &= ~(EINJ_VAL_DIFF);
+		val |= FIELD_PREP(EINJ_VAL_DIFF, val_diff);
+	}
+	if (err_group == 4) {
+		val &= ~(EINJ_VC_NUM);
+		val |= FIELD_PREP(EINJ_VC_NUM, vc_num);
+	}
+
+	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + ERR_INJ0_OFF + (0x4 * err_group), val);
+	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + ERR_INJ_ENABLE_REG, (0x1 << err_group));
+
+	kfree(kern_buf);
+	return count;
+}
+
+#define dwc_debugfs_create(name)			\
+debugfs_create_file(#name, 0644, rasdes_debug, pci,	\
+			&dbg_ ## name ## _fops)
+
+#define DWC_DEBUGFS_FOPS(name)					\
+static const struct file_operations dbg_ ## name ## _fops = {	\
+	.open = simple_open,				\
+	.read = name ## _read,				\
+	.write = name ## _write				\
+}
+
+DWC_DEBUGFS_FOPS(lane_detect);
+DWC_DEBUGFS_FOPS(rx_valid);
+
+static const struct file_operations dwc_pcie_counter_enable_ops = {
+	.open = simple_open,
+	.read = counter_enable_read,
+	.write = counter_enable_write,
+};
+
+static const struct file_operations dwc_pcie_counter_lane_ops = {
+	.open = simple_open,
+	.read = counter_lane_read,
+	.write = counter_lane_write,
+};
+
+static const struct file_operations dwc_pcie_counter_value_ops = {
+	.open = simple_open,
+	.read = counter_value_read,
+};
+
+static const struct file_operations dwc_pcie_err_inj_ops = {
+	.open = simple_open,
+	.write = err_inj_write,
+};
+
+void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci)
+{
+	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
+
+	debugfs_remove_recursive(rinfo->rasdes_dir);
+	mutex_destroy(&rinfo->reg_lock);
+}
+
+int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci)
+{
+	struct dentry *dir, *rasdes_debug, *rasdes_err_inj, *rasdes_event_counter, *rasdes_events;
+	struct dwc_pcie_rasdes_info *rasdes_info;
+	struct dwc_pcie_rasdes_priv *priv_tmp;
+	char dirname[DWC_DEBUGFS_BUF_MAX];
+	struct device *dev = pci->dev;
+	int ras_cap, i, ret;
+
+	ras_cap = dw_pcie_find_vsec_capability(pci, DW_PCIE_VSEC_EXT_CAP_RAS_DES);
+	if (!ras_cap) {
+		dev_dbg(dev, "No RASDES capability available\n");
+		return -ENODEV;
+	}
+
+	rasdes_info = devm_kzalloc(dev, sizeof(*rasdes_info), GFP_KERNEL);
+	if (!rasdes_info)
+		return -ENOMEM;
+
+	/* Create main directory for each platform driver */
+	snprintf(dirname, DWC_DEBUGFS_BUF_MAX, "dwc_pcie_%s", dev_name(dev));
+	dir = debugfs_create_dir(dirname, NULL);
+	if (IS_ERR(dir))
+		return PTR_ERR(dir);
+
+	/* Create subdirectories for Debug, Error injection, Statistics */
+	rasdes_debug = debugfs_create_dir("rasdes_debug", dir);
+	rasdes_err_inj = debugfs_create_dir("rasdes_err_inj", dir);
+	rasdes_event_counter = debugfs_create_dir("rasdes_event_counter", dir);
+
+	mutex_init(&rasdes_info->reg_lock);
+	rasdes_info->ras_cap_offset = ras_cap;
+	rasdes_info->rasdes_dir = dir;
+	pci->rasdes_info = rasdes_info;
+
+	/* Create debugfs files for Debug subdirectory */
+	dwc_debugfs_create(lane_detect);
+	dwc_debugfs_create(rx_valid);
+
+	/* Create debugfs files for Error injection subdirectory */
+	for (i = 0; i < ARRAY_SIZE(err_inj_list); i++) {
+		priv_tmp = devm_kzalloc(dev, sizeof(*priv_tmp), GFP_KERNEL);
+		if (!priv_tmp) {
+			ret = -ENOMEM;
+			goto err_deinit;
+		}
+
+		priv_tmp->idx = i;
+		priv_tmp->pci = pci;
+		debugfs_create_file(err_inj_list[i].name, 0200, rasdes_err_inj, priv_tmp,
+				    &dwc_pcie_err_inj_ops);
+	}
+
+	/* Create debugfs files for Statistical counter subdirectory */
+	for (i = 0; i < ARRAY_SIZE(event_list); i++) {
+		priv_tmp = devm_kzalloc(dev, sizeof(*priv_tmp), GFP_KERNEL);
+		if (!priv_tmp) {
+			ret = -ENOMEM;
+			goto err_deinit;
+		}
+
+		priv_tmp->idx = i;
+		priv_tmp->pci = pci;
+		rasdes_events = debugfs_create_dir(event_list[i].name, rasdes_event_counter);
+		if (event_list[i].group_no == 0 || event_list[i].group_no == 4) {
+			debugfs_create_file("lane_select", 0644, rasdes_events,
+					    priv_tmp, &dwc_pcie_counter_lane_ops);
+		}
+		debugfs_create_file("counter_value", 0444, rasdes_events, priv_tmp,
+				    &dwc_pcie_counter_value_ops);
+		debugfs_create_file("counter_enable", 0644, rasdes_events, priv_tmp,
+				    &dwc_pcie_counter_enable_ops);
+	}
+
+	return 0;
+
+err_deinit:
+	dwc_pcie_rasdes_debugfs_deinit(pci);
+	return ret;
+}
diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.h b/drivers/pci/controller/dwc/pcie-designware-debugfs.h
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 98a057820bc7..ed0f26d69626 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -260,6 +260,8 @@
 
 #define PCIE_RAS_DES_EVENT_COUNTER_DATA		0xc
 
+#define DW_PCIE_VSEC_EXT_CAP_RAS_DES		0x2
+
 /*
  * The default address offset between dbi_base and atu_base. Root controller
  * drivers are not required to initialize atu_base if the offset matches this
@@ -463,6 +465,7 @@ struct dw_pcie {
 	struct reset_control_bulk_data	core_rsts[DW_PCIE_NUM_CORE_RSTS];
 	struct gpio_desc		*pe_rst;
 	bool			suspended;
+	void			*rasdes_info;
 };
 
 #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
@@ -796,4 +799,18 @@ dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
 	return NULL;
 }
 #endif
+
+#ifdef CONFIG_PCIE_DW_DEBUGFS
+int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci);
+void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci);
+#else
+static inline int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci)
+{
+	return 0;
+}
+static inline void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci)
+{
+}
+#endif
+
 #endif /* _PCIE_DESIGNWARE_H */
-- 
2.17.1

Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[kernel test robot]

Hi Shradha,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus mani-mhi/mhi-next linus/master v6.13-rc1 next-20241205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shradha-Todi/PCI-dwc-Add-support-for-vendor-specific-capability-search/20241206-163620
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20241206074456.17401-2-shradha.t%40samsung.com
patch subject: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
config: i386-buildonly-randconfig-006 (https://download.01.org/0day-ci/archive/20241206/202412061940.WSlxZ8i1-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241206/202412061940.WSlxZ8i1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412061940.WSlxZ8i1-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/pci/controller/dwc/pcie-designware.c: In function 'dw_pcie_find_vsec_capability':
>> drivers/pci/controller/dwc/pcie-designware.c:285:16: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
     285 |         while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
         |                ^~~~


vim +285 drivers/pci/controller/dwc/pcie-designware.c

   279	
   280	u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
   281	{
   282		u16 vsec = 0;
   283		u32 header;
   284	
 > 285		while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
   286						PCI_EXT_CAP_ID_VNDR)) {
   287			header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
   288			if (PCI_VNDR_HEADER_ID(header) == vsec_cap)
   289				return vsec;
   290		}
   291	
   292		return 0;
   293	}
   294	EXPORT_SYMBOL_GPL(dw_pcie_find_vsec_capability);
   295	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[kernel test robot]

Hi Shradha,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus mani-mhi/mhi-next linus/master v6.13-rc1 next-20241205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shradha-Todi/PCI-dwc-Add-support-for-vendor-specific-capability-search/20241206-163620
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20241206074456.17401-2-shradha.t%40samsung.com
patch subject: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
config: arm64-randconfig-003-20241206 (https://download.01.org/0day-ci/archive/20241206/202412062046.0XR2e2V6-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241206/202412062046.0XR2e2V6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412062046.0XR2e2V6-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/pci/controller/dwc/pcie-designware.c:15:
   In file included from include/linux/dma/edma.h:13:
   In file included from include/linux/dmaengine.h:12:
   In file included from include/linux/scatterlist.h:8:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/dwc/pcie-designware.c:285:14: warning: using the result of an assignment as a condition without parentheses [-Wparentheses]
     285 |         while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
         |                ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     286 |                                         PCI_EXT_CAP_ID_VNDR)) {
         |                                         ~~~~~~~~~~~~~~~~~~~~
   drivers/pci/controller/dwc/pcie-designware.c:285:14: note: place parentheses around the assignment to silence this warning
     285 |         while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
         |                     ^
         |                (
     286 |                                         PCI_EXT_CAP_ID_VNDR)) {
         |                                                             
         |                                                             )
   drivers/pci/controller/dwc/pcie-designware.c:285:14: note: use '==' to turn this assignment into an equality comparison
     285 |         while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
         |                     ^
         |                     ==
   5 warnings generated.


vim +285 drivers/pci/controller/dwc/pcie-designware.c

   279	
   280	u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
   281	{
   282		u16 vsec = 0;
   283		u32 header;
   284	
 > 285		while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
   286						PCI_EXT_CAP_ID_VNDR)) {
   287			header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
   288			if (PCI_VNDR_HEADER_ID(header) == vsec_cap)
   289				return vsec;
   290		}
   291	
   292		return 0;
   293	}
   294	EXPORT_SYMBOL_GPL(dw_pcie_find_vsec_capability);
   295	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Bjorn Helgaas]

On Fri, Dec 06, 2024 at 01:14:55PM +0530, Shradha Todi wrote:
> Add vendor specific extended configuration space capability search API
> using struct dw_pcie pointer for DW controllers.
> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 16 ++++++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.h |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 6d6cbc8b5b2c..41230c5e4a53 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
>  	return 0;
>  }
>  
> +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)

To make sure that we find a VSEC ID that corresponds to the expected
vendor, I think this interface needs to be the same as
pci_find_vsec_capability().  In particular, it needs to take a "u16
vendor" and a "u16 vsec_cap".

(pci_find_vsec_capability() takes an "int cap", but I don't think
that's quite right).

> +{
> +	u16 vsec = 0;
> +	u32 header;
> +
> +	while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
> +					PCI_EXT_CAP_ID_VNDR)) {
> +		header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
> +		if (PCI_VNDR_HEADER_ID(header) == vsec_cap)
> +			return vsec;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_find_vsec_capability);
> +
>  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap)
>  {
>  	return dw_pcie_find_next_ext_capability(pci, 0, cap);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 347ab74ac35a..98a057820bc7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -476,6 +476,7 @@ void dw_pcie_version_detect(struct dw_pcie *pci);
>  
>  u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
>  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap);
> +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap);
>  
>  int dw_pcie_read(void __iomem *addr, int size, u32 *val);
>  int dw_pcie_write(void __iomem *addr, int size, u32 val);
> -- 
> 2.17.1
> 

Re: [PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support in DWC

[Fan Ni]

On Fri, Dec 06, 2024 at 01:14:56PM +0530, Shradha Todi wrote:
> Add support to use the RASDES feature of DesignWare PCIe controller
> using debugfs entries.
> 
> RASDES is a vendor specific extended PCIe capability which reads the
> current hardware internal state of PCIe device. Following primary
> features are provided to userspace via debugfs:
> - Debug registers
> - Error injection
> - Statistical counters

I think this patch can break into several to make it easier to
review.
For example, it can be divided by the three features list above, with
the documentation change coming last as a separate patch.

Some minor comments inline.

> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  Documentation/ABI/testing/debugfs-dwc-pcie    | 143 +++++
>  drivers/pci/controller/dwc/Kconfig            |  11 +
>  drivers/pci/controller/dwc/Makefile           |   1 +
>  .../controller/dwc/pcie-designware-debugfs.c  | 544 ++++++++++++++++++
>  .../controller/dwc/pcie-designware-debugfs.h  |   0
>  drivers/pci/controller/dwc/pcie-designware.h  |  17 +
>  6 files changed, 716 insertions(+)
>  create mode 100644 Documentation/ABI/testing/debugfs-dwc-pcie
>  create mode 100644 drivers/pci/controller/dwc/pcie-designware-debugfs.c
>  create mode 100644 drivers/pci/controller/dwc/pcie-designware-debugfs.h
> 
> diff --git a/Documentation/ABI/testing/debugfs-dwc-pcie b/Documentation/ABI/testing/debugfs-dwc-pcie
> new file mode 100644
> index 000000000000..7da73ac8d40c
> --- /dev/null
> +++ b/Documentation/ABI/testing/debugfs-dwc-pcie
> @@ -0,0 +1,143 @@
> +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/lane_detect
> +Date:		December 2024
> +Contact:	Shradha Todi <shradha.t@samsung.com>
> +Description:	(RW) Write the lane number to be checked for detection.	Read
> +		will dump whether PHY indicates receiver detection on the
> +		selected lane.
> +
> +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/rx_valid
> +Date:		December 2024
> +Contact:	Shradha Todi <shradha.t@samsung.com>
> +Description:	(RW) Write the lane number to be checked as valid or invalid. Read
> +		will dump the status of PIPE RXVALID signal of the selected lane.
> +
> +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_event_counters/<event>/counter_enable

I think "counter_enable" can be "enable" since the upper directory already
shows it is for event counter.

> +Date:		December 2024
> +Contact:	Shradha Todi <shradha.t@samsung.com>
> +Description:	rasdes_event_counters is the directory which can be used to collect
> +		statistical data about the number of times a certain event has occurred
> +		in the controller. The list of possible events are:
> +
> +		1) EBUF Overflow
> +		2) EBUF Underrun
> +		3) Decode Error
> +		4) Running Disparity Error
> +		5) SKP OS Parity Error
> +		6) SYNC Header Error
> +		7) Rx Valid De-assertion
> +		8) CTL SKP OS Parity Error
> +		9) 1st Retimer Parity Error
> +		10) 2nd Retimer Parity Error
> +		11) Margin CRC and Parity Error
> +		12) Detect EI Infer
> +		13) Receiver Error
> +		14) RX Recovery Req
> +		15) N_FTS Timeout
> +		16) Framing Error
> +		17) Deskew Error
> +		18) Framing Error In L0
> +		19) Deskew Uncompleted Error
> +		20) Bad TLP
> +		21) LCRC Error
> +		22) Bad DLLP
> +		23) Replay Number Rollover
> +		24) Replay Timeout
> +		25) Rx Nak DLLP
> +		26) Tx Nak DLLP
> +		27) Retry TLP
> +		28) FC Timeout
> +		29) Poisoned TLP
> +		30) ECRC Error
> +		31) Unsupported Request
> +		32) Completer Abort
> +		33) Completion Timeout
> +		34) EBUF SKP Add
> +		35) EBUF SKP Del
> +
> +		counter_enable is RW. Write 1 to enable the event counter and write 0 to
> +		disable the event counter. Read will dump whether the counter is currently
> +		enabled	or disabled.
...
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index b6d6778b0698..9ab8d724fe0d 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -6,6 +6,17 @@ menu "DesignWare-based PCIe controllers"
>  config PCIE_DW
>  	bool
>  
> +config PCIE_DW_DEBUGFS
> +	default y
Why we need to enable it by default?

Fan
> +	depends on DEBUG_FS
> +	depends on PCIE_DW_HOST || PCIE_DW_EP
> +	bool "DWC PCIe debugfs entries"
> +	help
> +	  Enables debugfs entries for the DWC PCIe Controller.
> +	  These entries make use of the RAS features in the DW
> +	  controller to help in debug, error injection and statistical
> +	  counters
> +
>  config PCIE_DW_HOST
>  	bool
>  	select PCIE_DW
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index a8308d9ea986..54565eedc52c 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> +obj-$(CONFIG_PCIE_DW_DEBUGFS) += pcie-designware-debugfs.o
>  obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>  obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.c b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> new file mode 100644
> index 000000000000..a93e29993f75
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> @@ -0,0 +1,544 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Synopsys DesignWare PCIe controller debugfs driver
> + *
> + * Copyright (C) 2024 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com
> + *
> + * Author: Shradha Todi <shradha.t@samsung.com>
> + */
> +
> +#include <linux/debugfs.h>
> +
> +#include "pcie-designware.h"
> +
> +#define SD_STATUS_L1LANE_REG		0xb0
> +#define PIPE_DETECT_LANE		BIT(17)
> +#define PIPE_RXVALID			BIT(18)
> +#define LANE_SELECT			GENMASK(3, 0)
> +
> +#define RAS_DES_EVENT_COUNTER_CTRL_REG	0x8
> +#define EVENT_COUNTER_GROUP_SELECT	GENMASK(27, 24)
> +#define EVENT_COUNTER_EVENT_SELECT	GENMASK(23, 16)
> +#define EVENT_COUNTER_LANE_SELECT	GENMASK(11, 8)
> +#define EVENT_COUNTER_STATUS		BIT(7)
> +#define EVENT_COUNTER_ENABLE		GENMASK(4, 2)
> +#define PER_EVENT_OFF			0x1
> +#define PER_EVENT_ON			0x3
> +
> +#define RAS_DES_EVENT_COUNTER_DATA_REG	0xc
> +
> +#define ERR_INJ_ENABLE_REG		0x30
> +#define ERR_INJ0_OFF			0x34
> +#define EINJ_COUNT			GENMASK(7, 0)
> +#define EINJ_TYPE_SHIFT			8
> +#define EINJ0_TYPE			GENMASK(11, 8)
> +#define EINJ1_TYPE			BIT(8)
> +#define EINJ2_TYPE			GENMASK(9, 8)
> +#define EINJ3_TYPE			GENMASK(10, 8)
> +#define EINJ4_TYPE			GENMASK(10, 8)
> +#define EINJ5_TYPE			BIT(8)
> +#define EINJ_VC_NUM			GENMASK(14, 12)
> +#define EINJ_VAL_DIFF			GENMASK(28, 16)
> +
> +#define DWC_DEBUGFS_BUF_MAX		128
> +
> +/**
> + * struct dwc_pcie_rasdes_info - Stores controller common information
> + * @ras_cap_offset: RAS DES vendor specific extended capability offset
> + * @reg_lock: Mutex used for RASDES shadow event registers
> + * @rasdes_dir: Top level debugfs directory entry
> + *
> + * Any parameter constant to all files of the debugfs hierarchy for a single controller
> + * will be stored in this struct. It is allocated and assigned to controller specific
> + * struct dw_pcie during initialization.
> + */
> +struct dwc_pcie_rasdes_info {
> +	u32 ras_cap_offset;
> +	struct mutex reg_lock;
> +	struct dentry *rasdes_dir;
> +};
> +
> +/**
> + * struct dwc_pcie_rasdes_priv - Stores file specific private data information
> + * @pci: Reference to the dw_pcie structure
> + * @idx: Index to point to specific file related information in array of structs
> + *
> + * All debugfs files will have this struct as its private data.
> + */
> +struct dwc_pcie_rasdes_priv {
> +	struct dw_pcie *pci;
> +	int idx;
> +};
> +
> +/**
> + * struct dwc_pcie_event_counter - Store details about each event counter supported in DWC RASDES
> + * @name: Name of the error counter
> + * @group_no: Group number that the event belongs to. Value ranges from 0 - 4
> + * @event_no: Event number of the particular event. Value ranges from -
> + *		Group 0: 0 - 10
> + *		Group 1: 5 - 13
> + *		Group 2: 0 - 7
> + *		Group 3: 0 - 5
> + *		Group 4: 0 - 1
> + */
> +struct dwc_pcie_event_counter {
> +	const char *name;
> +	u32 group_no;
> +	u32 event_no;
> +};
> +
> +static const struct dwc_pcie_event_counter event_list[] = {
> +	{"ebuf_overflow", 0x0, 0x0},
> +	{"ebuf_underrun", 0x0, 0x1},
> +	{"decode_err", 0x0, 0x2},
> +	{"running_disparity_err", 0x0, 0x3},
> +	{"skp_os_parity_err", 0x0, 0x4},
> +	{"sync_header_err", 0x0, 0x5},
> +	{"rx_valid_deassertion", 0x0, 0x6},
> +	{"ctl_skp_os_parity_err", 0x0, 0x7},
> +	{"retimer_parity_err_1st", 0x0, 0x8},
> +	{"retimer_parity_err_2nd", 0x0, 0x9},
> +	{"margin_crc_parity_err", 0x0, 0xA},
> +	{"detect_ei_infer", 0x1, 0x5},
> +	{"receiver_err", 0x1, 0x6},
> +	{"rx_recovery_req", 0x1, 0x7},
> +	{"n_fts_timeout", 0x1, 0x8},
> +	{"framing_err", 0x1, 0x9},
> +	{"deskew_err", 0x1, 0xa},
> +	{"framing_err_in_l0", 0x1, 0xc},
> +	{"deskew_uncompleted_err", 0x1, 0xd},
> +	{"bad_tlp", 0x2, 0x0},
> +	{"lcrc_err", 0x2, 0x1},
> +	{"bad_dllp", 0x2, 0x2},
> +	{"replay_num_rollover", 0x2, 0x3},
> +	{"replay_timeout", 0x2, 0x4},
> +	{"rx_nak_dllp", 0x2, 0x5},
> +	{"tx_nak_dllp", 0x2, 0x6},
> +	{"retry_tlp", 0x2, 0x7},
> +	{"fc_timeout", 0x3, 0x0},
> +	{"poisoned_tlp", 0x3, 0x1},
> +	{"ecrc_error", 0x3, 0x2},
> +	{"unsupported_request", 0x3, 0x3},
> +	{"completer_abort", 0x3, 0x4},
> +	{"completion_timeout", 0x3, 0x5},
> +	{"ebuf_skp_add", 0x4, 0x0},
> +	{"ebuf_skp_del", 0x4, 0x1},
> +};
> +
> +/**
> + * struct dwc_pcie_err_inj - Store details about each error injection supported by DWC RASDES
> + * @name: Name of the error that can be injected
> + * @err_inj_group: Group number to which the error belongs to. Value can range from 0 - 5
> + * @err_inj_type: Each group can have multiple types of error
> + */
> +struct dwc_pcie_err_inj {
> +	const char *name;
> +	u32 err_inj_group;
> +	u32 err_inj_type;
> +};
> +
> +static const struct dwc_pcie_err_inj err_inj_list[] = {
> +	{"tx_lcrc", 0x0, 0x0},
> +	{"b16_crc_dllp", 0x0, 0x1},
> +	{"b16_crc_upd_fc", 0x0, 0x2},
> +	{"tx_ecrc", 0x0, 0x3},
> +	{"fcrc_tlp", 0x0, 0x4},
> +	{"parity_tsos", 0x0, 0x5},
> +	{"parity_skpos", 0x0, 0x6},
> +	{"rx_lcrc", 0x0, 0x8},
> +	{"rx_ecrc", 0x0, 0xb},
> +	{"tlp_err_seq", 0x1, 0x0},
> +	{"ack_nak_dllp_seq", 0x1, 0x1},
> +	{"ack_nak_dllp", 0x2, 0x0},
> +	{"upd_fc_dllp", 0x2, 0x1},
> +	{"nak_dllp", 0x2, 0x2},
> +	{"inv_sync_hdr_sym", 0x3, 0x0},
> +	{"com_pad_ts1", 0x3, 0x1},
> +	{"com_pad_ts2", 0x3, 0x2},
> +	{"com_fts", 0x3, 0x3},
> +	{"com_idl", 0x3, 0x4},
> +	{"end_edb", 0x3, 0x5},
> +	{"stp_sdp", 0x3, 0x6},
> +	{"com_skp", 0x3, 0x7},
> +	{"posted_tlp_hdr", 0x4, 0x0},
> +	{"non_post_tlp_hdr", 0x4, 0x1},
> +	{"cmpl_tlp_hdr", 0x4, 0x2},
> +	{"posted_tlp_data", 0x4, 0x4},
> +	{"non_post_tlp_data", 0x4, 0x5},
> +	{"cmpl_tlp_data", 0x4, 0x6},
> +	{"duplicate_dllp", 0x5, 0x0},
> +	{"nullified_tlp", 0x5, 0x1},
> +};
> +
> +static const u32 err_inj_type_mask[] = {
> +	EINJ0_TYPE,
> +	EINJ1_TYPE,
> +	EINJ2_TYPE,
> +	EINJ3_TYPE,
> +	EINJ4_TYPE,
> +	EINJ5_TYPE,
> +};
> +
> +static ssize_t lane_detect_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct dw_pcie *pci = file->private_data;
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> +	ssize_t off = 0;
> +	u32 val;
> +
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
> +	val = FIELD_GET(PIPE_DETECT_LANE, val);
> +	if (val)
> +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane Detected\n");
> +	else
> +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane Undetected\n");
> +
> +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> +}
> +
> +static ssize_t lane_detect_write(struct file *file, const char __user *buf,
> +				 size_t count, loff_t *ppos)
> +{
> +	struct dw_pcie *pci = file->private_data;
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +	u32 lane, val;
> +
> +	val = kstrtou32_from_user(buf, count, 0, &lane);
> +	if (val)
> +		return val;
> +
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
> +	val &= ~(LANE_SELECT);
> +	val |= FIELD_PREP(LANE_SELECT, lane);
> +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG, val);
> +
> +	return count;
> +}
> +
> +static ssize_t rx_valid_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct dw_pcie *pci = file->private_data;
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> +	ssize_t off = 0;
> +	u32 val;
> +
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
> +	val = FIELD_GET(PIPE_RXVALID, val);
> +	if (val)
> +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "RX Valid\n");
> +	else
> +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "RX Invalid\n");
> +
> +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> +}
> +
> +static ssize_t rx_valid_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
> +{
> +	return lane_detect_write(file, buf, count, ppos);
> +}
> +
> +static void set_event_number(struct dwc_pcie_rasdes_priv *pdata, struct dw_pcie *pci,
> +			     struct dwc_pcie_rasdes_info *rinfo)
> +{
> +	u32 val;
> +
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> +	val &= ~EVENT_COUNTER_ENABLE;
> +	val &= ~(EVENT_COUNTER_GROUP_SELECT | EVENT_COUNTER_EVENT_SELECT);
> +	val |= FIELD_PREP(EVENT_COUNTER_GROUP_SELECT, event_list[pdata->idx].group_no);
> +	val |= FIELD_PREP(EVENT_COUNTER_EVENT_SELECT, event_list[pdata->idx].event_no);
> +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
> +}
> +
> +static ssize_t counter_enable_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> +	struct dw_pcie *pci = pdata->pci;
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> +	ssize_t off = 0;
> +	u32 val;
> +
> +	mutex_lock(&rinfo->reg_lock);
> +	set_event_number(pdata, pci, rinfo);
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> +	mutex_unlock(&rinfo->reg_lock);
> +	val = FIELD_GET(EVENT_COUNTER_STATUS, val);
> +	if (val)
> +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter Enabled\n");
> +	else
> +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter Disabled\n");
> +
> +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> +}
> +
> +static ssize_t counter_enable_write(struct file *file, const char __user *buf,
> +				    size_t count, loff_t *ppos)
> +{
> +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> +	struct dw_pcie *pci = pdata->pci;
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +	u32 val, enable;
> +
> +	val = kstrtou32_from_user(buf, count, 0, &enable);
> +	if (val)
> +		return val;
> +
> +	mutex_lock(&rinfo->reg_lock);
> +	set_event_number(pdata, pci, rinfo);
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> +	if (enable)
> +		val |= FIELD_PREP(EVENT_COUNTER_ENABLE, PER_EVENT_ON);
> +	else
> +		val |= FIELD_PREP(EVENT_COUNTER_ENABLE, PER_EVENT_OFF);
> +
> +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
> +	mutex_unlock(&rinfo->reg_lock);
> +
> +	return count;
> +}
> +
> +static ssize_t counter_lane_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> +	struct dw_pcie *pci = pdata->pci;
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> +	ssize_t off = 0;
> +	u32 val;
> +
> +	mutex_lock(&rinfo->reg_lock);
> +	set_event_number(pdata, pci, rinfo);
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> +	mutex_unlock(&rinfo->reg_lock);
> +	val = FIELD_GET(EVENT_COUNTER_LANE_SELECT, val);
> +	off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane: %d\n", val);
> +
> +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> +}
> +
> +static ssize_t counter_lane_write(struct file *file, const char __user *buf,
> +				  size_t count, loff_t *ppos)
> +{
> +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> +	struct dw_pcie *pci = pdata->pci;
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +	u32 val, lane;
> +
> +	val = kstrtou32_from_user(buf, count, 0, &lane);
> +	if (val)
> +		return val;
> +
> +	mutex_lock(&rinfo->reg_lock);
> +	set_event_number(pdata, pci, rinfo);
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> +	val &= ~(EVENT_COUNTER_LANE_SELECT);
> +	val |= FIELD_PREP(EVENT_COUNTER_LANE_SELECT, lane);
> +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
> +	mutex_unlock(&rinfo->reg_lock);
> +
> +	return count;
> +}
> +
> +static ssize_t counter_value_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> +	struct dw_pcie *pci = pdata->pci;
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> +	ssize_t off = 0;
> +	u32 val;
> +
> +	mutex_lock(&rinfo->reg_lock);
> +	set_event_number(pdata, pci, rinfo);
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_DATA_REG);
> +	mutex_unlock(&rinfo->reg_lock);
> +	off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter value: %d\n", val);
> +
> +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> +}
> +
> +static ssize_t err_inj_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> +	struct dw_pcie *pci = pdata->pci;
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +	u32 val, counter, vc_num, err_group, type_mask;
> +	int val_diff = 0;
> +	char *kern_buf;
> +
> +	err_group = err_inj_list[pdata->idx].err_inj_group;
> +	type_mask = err_inj_type_mask[err_group];
> +
> +	kern_buf = memdup_user_nul(buf, count);
> +	if (IS_ERR(kern_buf))
> +		return PTR_ERR(kern_buf);
> +
> +	if (err_group == 4) {
> +		val = sscanf(kern_buf, "%u %d %u", &counter, &val_diff, &vc_num);
> +		if ((val != 3) || (val_diff < -4095 || val_diff > 4095)) {
> +			kfree(kern_buf);
> +			return -EINVAL;
> +		}
> +	} else if (err_group == 1) {
> +		val = sscanf(kern_buf, "%u %d", &counter, &val_diff);
> +		if ((val != 2) || (val_diff < -4095 || val_diff > 4095)) {
> +			kfree(kern_buf);
> +			return -EINVAL;
> +		}
> +	} else {
> +		val = kstrtou32(kern_buf, 0, &counter);
> +		if (val) {
> +			kfree(kern_buf);
> +			return val;
> +		}
> +	}
> +
> +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + ERR_INJ0_OFF + (0x4 * err_group));
> +	val &= ~(type_mask | EINJ_COUNT);
> +	val |= ((err_inj_list[pdata->idx].err_inj_type << EINJ_TYPE_SHIFT) & type_mask);
> +	val |= FIELD_PREP(EINJ_COUNT, counter);
> +
> +	if (err_group == 1 || err_group == 4) {
> +		val &= ~(EINJ_VAL_DIFF);
> +		val |= FIELD_PREP(EINJ_VAL_DIFF, val_diff);
> +	}
> +	if (err_group == 4) {
> +		val &= ~(EINJ_VC_NUM);
> +		val |= FIELD_PREP(EINJ_VC_NUM, vc_num);
> +	}
> +
> +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + ERR_INJ0_OFF + (0x4 * err_group), val);
> +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + ERR_INJ_ENABLE_REG, (0x1 << err_group));
> +
> +	kfree(kern_buf);
> +	return count;
> +}
> +
> +#define dwc_debugfs_create(name)			\
> +debugfs_create_file(#name, 0644, rasdes_debug, pci,	\
> +			&dbg_ ## name ## _fops)
> +
> +#define DWC_DEBUGFS_FOPS(name)					\
> +static const struct file_operations dbg_ ## name ## _fops = {	\
> +	.open = simple_open,				\
> +	.read = name ## _read,				\
> +	.write = name ## _write				\
> +}
> +
> +DWC_DEBUGFS_FOPS(lane_detect);
> +DWC_DEBUGFS_FOPS(rx_valid);
> +
> +static const struct file_operations dwc_pcie_counter_enable_ops = {
> +	.open = simple_open,
> +	.read = counter_enable_read,
> +	.write = counter_enable_write,
> +};
> +
> +static const struct file_operations dwc_pcie_counter_lane_ops = {
> +	.open = simple_open,
> +	.read = counter_lane_read,
> +	.write = counter_lane_write,
> +};
> +
> +static const struct file_operations dwc_pcie_counter_value_ops = {
> +	.open = simple_open,
> +	.read = counter_value_read,
> +};
> +
> +static const struct file_operations dwc_pcie_err_inj_ops = {
> +	.open = simple_open,
> +	.write = err_inj_write,
> +};
> +
> +void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci)
> +{
> +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> +
> +	debugfs_remove_recursive(rinfo->rasdes_dir);
> +	mutex_destroy(&rinfo->reg_lock);
> +}
> +
> +int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci)
> +{
> +	struct dentry *dir, *rasdes_debug, *rasdes_err_inj, *rasdes_event_counter, *rasdes_events;
> +	struct dwc_pcie_rasdes_info *rasdes_info;
> +	struct dwc_pcie_rasdes_priv *priv_tmp;
> +	char dirname[DWC_DEBUGFS_BUF_MAX];
> +	struct device *dev = pci->dev;
> +	int ras_cap, i, ret;
> +
> +	ras_cap = dw_pcie_find_vsec_capability(pci, DW_PCIE_VSEC_EXT_CAP_RAS_DES);
> +	if (!ras_cap) {
> +		dev_dbg(dev, "No RASDES capability available\n");
> +		return -ENODEV;
> +	}
> +
> +	rasdes_info = devm_kzalloc(dev, sizeof(*rasdes_info), GFP_KERNEL);
> +	if (!rasdes_info)
> +		return -ENOMEM;
> +
> +	/* Create main directory for each platform driver */
> +	snprintf(dirname, DWC_DEBUGFS_BUF_MAX, "dwc_pcie_%s", dev_name(dev));
> +	dir = debugfs_create_dir(dirname, NULL);
> +	if (IS_ERR(dir))
> +		return PTR_ERR(dir);
> +
> +	/* Create subdirectories for Debug, Error injection, Statistics */
> +	rasdes_debug = debugfs_create_dir("rasdes_debug", dir);
> +	rasdes_err_inj = debugfs_create_dir("rasdes_err_inj", dir);
> +	rasdes_event_counter = debugfs_create_dir("rasdes_event_counter", dir);
> +
> +	mutex_init(&rasdes_info->reg_lock);
> +	rasdes_info->ras_cap_offset = ras_cap;
> +	rasdes_info->rasdes_dir = dir;
> +	pci->rasdes_info = rasdes_info;
> +
> +	/* Create debugfs files for Debug subdirectory */
> +	dwc_debugfs_create(lane_detect);
> +	dwc_debugfs_create(rx_valid);
> +
> +	/* Create debugfs files for Error injection subdirectory */
> +	for (i = 0; i < ARRAY_SIZE(err_inj_list); i++) {
> +		priv_tmp = devm_kzalloc(dev, sizeof(*priv_tmp), GFP_KERNEL);
> +		if (!priv_tmp) {
> +			ret = -ENOMEM;
> +			goto err_deinit;
> +		}
> +
> +		priv_tmp->idx = i;
> +		priv_tmp->pci = pci;
> +		debugfs_create_file(err_inj_list[i].name, 0200, rasdes_err_inj, priv_tmp,
> +				    &dwc_pcie_err_inj_ops);
> +	}
> +
> +	/* Create debugfs files for Statistical counter subdirectory */
> +	for (i = 0; i < ARRAY_SIZE(event_list); i++) {
> +		priv_tmp = devm_kzalloc(dev, sizeof(*priv_tmp), GFP_KERNEL);
> +		if (!priv_tmp) {
> +			ret = -ENOMEM;
> +			goto err_deinit;
> +		}
> +
> +		priv_tmp->idx = i;
> +		priv_tmp->pci = pci;
> +		rasdes_events = debugfs_create_dir(event_list[i].name, rasdes_event_counter);
> +		if (event_list[i].group_no == 0 || event_list[i].group_no == 4) {
> +			debugfs_create_file("lane_select", 0644, rasdes_events,
> +					    priv_tmp, &dwc_pcie_counter_lane_ops);
> +		}
> +		debugfs_create_file("counter_value", 0444, rasdes_events, priv_tmp,
> +				    &dwc_pcie_counter_value_ops);
> +		debugfs_create_file("counter_enable", 0644, rasdes_events, priv_tmp,
> +				    &dwc_pcie_counter_enable_ops);
> +	}
> +
> +	return 0;
> +
> +err_deinit:
> +	dwc_pcie_rasdes_debugfs_deinit(pci);
> +	return ret;
> +}
> diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.h b/drivers/pci/controller/dwc/pcie-designware-debugfs.h
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 98a057820bc7..ed0f26d69626 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -260,6 +260,8 @@
>  
>  #define PCIE_RAS_DES_EVENT_COUNTER_DATA		0xc
>  
> +#define DW_PCIE_VSEC_EXT_CAP_RAS_DES		0x2
> +
>  /*
>   * The default address offset between dbi_base and atu_base. Root controller
>   * drivers are not required to initialize atu_base if the offset matches this
> @@ -463,6 +465,7 @@ struct dw_pcie {
>  	struct reset_control_bulk_data	core_rsts[DW_PCIE_NUM_CORE_RSTS];
>  	struct gpio_desc		*pe_rst;
>  	bool			suspended;
> +	void			*rasdes_info;
>  };
>  
>  #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
> @@ -796,4 +799,18 @@ dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
>  	return NULL;
>  }
>  #endif
> +
> +#ifdef CONFIG_PCIE_DW_DEBUGFS
> +int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci);
> +void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci);
> +#else
> +static inline int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci)
> +{
> +	return 0;
> +}
> +static inline void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci)
> +{
> +}
> +#endif
> +
>  #endif /* _PCIE_DESIGNWARE_H */
> -- 
> 2.17.1
> 

-- 
Fan Ni (From gmail)

RE: [PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support in DWC

[Shradha Todi]

> -----Original Message-----
> From: Fan Ni <nifan.cxl@gmail.com>
> Sent: 10 December 2024 03:59
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com; quic_krichai@quicinc.com;
> gost.dev@samsung.com
> Subject: Re: [PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support in DWC
> 
> On Fri, Dec 06, 2024 at 01:14:56PM +0530, Shradha Todi wrote:
> > Add support to use the RASDES feature of DesignWare PCIe controller
> > using debugfs entries.
> >
> > RASDES is a vendor specific extended PCIe capability which reads the
> > current hardware internal state of PCIe device. Following primary
> > features are provided to userspace via debugfs:
> > - Debug registers
> > - Error injection
> > - Statistical counters
> 
> I think this patch can break into several to make it easier to review.
> For example, it can be divided by the three features list above, with the documentation change coming last as a
separate
> patch.
> 

Sure Fan. I have no issues in breaking this into smaller patches. Though I think the documentation
should go along with the implementation rather than a separate patch?
Anyway, I'll wait for some time for further review comments or if anyone has any objection to
splitting the patches before going for the next revision.

> Some minor comments inline.
> 
> >
> > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > ---
> >  Documentation/ABI/testing/debugfs-dwc-pcie    | 143 +++++
> >  drivers/pci/controller/dwc/Kconfig            |  11 +
> >  drivers/pci/controller/dwc/Makefile           |   1 +
> >  .../controller/dwc/pcie-designware-debugfs.c  | 544 ++++++++++++++++++
> >  .../controller/dwc/pcie-designware-debugfs.h  |   0
> >  drivers/pci/controller/dwc/pcie-designware.h  |  17 +
> >  6 files changed, 716 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/debugfs-dwc-pcie
> >  create mode 100644
> > drivers/pci/controller/dwc/pcie-designware-debugfs.c
> >  create mode 100644
> > drivers/pci/controller/dwc/pcie-designware-debugfs.h
> >
> > diff --git a/Documentation/ABI/testing/debugfs-dwc-pcie
> > b/Documentation/ABI/testing/debugfs-dwc-pcie
> > new file mode 100644
> > index 000000000000..7da73ac8d40c
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/debugfs-dwc-pcie
> > @@ -0,0 +1,143 @@
> > +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/lane_detect
> > +Date:		December 2024
> > +Contact:	Shradha Todi <shradha.t@samsung.com>
> > +Description:	(RW) Write the lane number to be checked for detection.	Read
> > +		will dump whether PHY indicates receiver detection on the
> > +		selected lane.
> > +
> > +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/rx_valid
> > +Date:		December 2024
> > +Contact:	Shradha Todi <shradha.t@samsung.com>
> > +Description:	(RW) Write the lane number to be checked as valid or invalid. Read
> > +		will dump the status of PIPE RXVALID signal of the selected lane.
> > +
> > +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_event_counters/<event>/counter_enable
> 
> I think "counter_enable" can be "enable" since the upper directory already shows it is for event counter.
>

Makes sense. I'll fix this in the next patchset.

> > +Date:		December 2024
> > +Contact:	Shradha Todi <shradha.t@samsung.com>
> > +Description:	rasdes_event_counters is the directory which can be used to collect
> > +		statistical data about the number of times a certain event has occurred
> > +		in the controller. The list of possible events are:
> > +
> > +		1) EBUF Overflow
> > +		2) EBUF Underrun
> > +		3) Decode Error
> > +		4) Running Disparity Error
> > +		5) SKP OS Parity Error
> > +		6) SYNC Header Error
> > +		7) Rx Valid De-assertion
> > +		8) CTL SKP OS Parity Error
> > +		9) 1st Retimer Parity Error
> > +		10) 2nd Retimer Parity Error
> > +		11) Margin CRC and Parity Error
> > +		12) Detect EI Infer
> > +		13) Receiver Error
> > +		14) RX Recovery Req
> > +		15) N_FTS Timeout
> > +		16) Framing Error
> > +		17) Deskew Error
> > +		18) Framing Error In L0
> > +		19) Deskew Uncompleted Error
> > +		20) Bad TLP
> > +		21) LCRC Error
> > +		22) Bad DLLP
> > +		23) Replay Number Rollover
> > +		24) Replay Timeout
> > +		25) Rx Nak DLLP
> > +		26) Tx Nak DLLP
> > +		27) Retry TLP
> > +		28) FC Timeout
> > +		29) Poisoned TLP
> > +		30) ECRC Error
> > +		31) Unsupported Request
> > +		32) Completer Abort
> > +		33) Completion Timeout
> > +		34) EBUF SKP Add
> > +		35) EBUF SKP Del
> > +
> > +		counter_enable is RW. Write 1 to enable the event counter and write 0 to
> > +		disable the event counter. Read will dump whether the counter is currently
> > +		enabled	or disabled.
> ...
> > diff --git a/drivers/pci/controller/dwc/Kconfig
> > b/drivers/pci/controller/dwc/Kconfig
> > index b6d6778b0698..9ab8d724fe0d 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -6,6 +6,17 @@ menu "DesignWare-based PCIe controllers"
> >  config PCIE_DW
> >  	bool
> >
> > +config PCIE_DW_DEBUGFS
> > +	default y
> Why we need to enable it by default?
> 

Actually the config depends on debug_fs, so the intention was that if debug_fs and atleast
one DW driver is enabled in the kernel, there should be no harm in enabling this. 

> Fan
> > +	depends on DEBUG_FS
> > +	depends on PCIE_DW_HOST || PCIE_DW_EP
> > +	bool "DWC PCIe debugfs entries"
> > +	help
> > +	  Enables debugfs entries for the DWC PCIe Controller.
> > +	  These entries make use of the RAS features in the DW
> > +	  controller to help in debug, error injection and statistical
> > +	  counters
> > +
> >  config PCIE_DW_HOST
> >  	bool
> >  	select PCIE_DW
> > diff --git a/drivers/pci/controller/dwc/Makefile
> > b/drivers/pci/controller/dwc/Makefile
> > index a8308d9ea986..54565eedc52c 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -1,5 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> > +obj-$(CONFIG_PCIE_DW_DEBUGFS) += pcie-designware-debugfs.o
> >  obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
> >  obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> >  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o diff --git
> > a/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> > b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> > new file mode 100644
> > index 000000000000..a93e29993f75
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> > @@ -0,0 +1,544 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Synopsys DesignWare PCIe controller debugfs driver
> > + *
> > + * Copyright (C) 2024 Samsung Electronics Co., Ltd.
> > + *		http://www.samsung.com
> > + *
> > + * Author: Shradha Todi <shradha.t@samsung.com>  */
> > +
> > +#include <linux/debugfs.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +#define SD_STATUS_L1LANE_REG		0xb0
> > +#define PIPE_DETECT_LANE		BIT(17)
> > +#define PIPE_RXVALID			BIT(18)
> > +#define LANE_SELECT			GENMASK(3, 0)
> > +
> > +#define RAS_DES_EVENT_COUNTER_CTRL_REG	0x8
> > +#define EVENT_COUNTER_GROUP_SELECT	GENMASK(27, 24)
> > +#define EVENT_COUNTER_EVENT_SELECT	GENMASK(23, 16)
> > +#define EVENT_COUNTER_LANE_SELECT	GENMASK(11, 8)
> > +#define EVENT_COUNTER_STATUS		BIT(7)
> > +#define EVENT_COUNTER_ENABLE		GENMASK(4, 2)
> > +#define PER_EVENT_OFF			0x1
> > +#define PER_EVENT_ON			0x3
> > +
> > +#define RAS_DES_EVENT_COUNTER_DATA_REG	0xc
> > +
> > +#define ERR_INJ_ENABLE_REG		0x30
> > +#define ERR_INJ0_OFF			0x34
> > +#define EINJ_COUNT			GENMASK(7, 0)
> > +#define EINJ_TYPE_SHIFT			8
> > +#define EINJ0_TYPE			GENMASK(11, 8)
> > +#define EINJ1_TYPE			BIT(8)
> > +#define EINJ2_TYPE			GENMASK(9, 8)
> > +#define EINJ3_TYPE			GENMASK(10, 8)
> > +#define EINJ4_TYPE			GENMASK(10, 8)
> > +#define EINJ5_TYPE			BIT(8)
> > +#define EINJ_VC_NUM			GENMASK(14, 12)
> > +#define EINJ_VAL_DIFF			GENMASK(28, 16)
> > +
> > +#define DWC_DEBUGFS_BUF_MAX		128
> > +
> > +/**
> > + * struct dwc_pcie_rasdes_info - Stores controller common information
> > + * @ras_cap_offset: RAS DES vendor specific extended capability
> > +offset
> > + * @reg_lock: Mutex used for RASDES shadow event registers
> > + * @rasdes_dir: Top level debugfs directory entry
> > + *
> > + * Any parameter constant to all files of the debugfs hierarchy for a
> > +single controller
> > + * will be stored in this struct. It is allocated and assigned to
> > +controller specific
> > + * struct dw_pcie during initialization.
> > + */
> > +struct dwc_pcie_rasdes_info {
> > +	u32 ras_cap_offset;
> > +	struct mutex reg_lock;
> > +	struct dentry *rasdes_dir;
> > +};
> > +
> > +/**
> > + * struct dwc_pcie_rasdes_priv - Stores file specific private data
> > +information
> > + * @pci: Reference to the dw_pcie structure
> > + * @idx: Index to point to specific file related information in array
> > +of structs
> > + *
> > + * All debugfs files will have this struct as its private data.
> > + */
> > +struct dwc_pcie_rasdes_priv {
> > +	struct dw_pcie *pci;
> > +	int idx;
> > +};
> > +
> > +/**
> > + * struct dwc_pcie_event_counter - Store details about each event
> > +counter supported in DWC RASDES
> > + * @name: Name of the error counter
> > + * @group_no: Group number that the event belongs to. Value ranges
> > +from 0 - 4
> > + * @event_no: Event number of the particular event. Value ranges from -
> > + *		Group 0: 0 - 10
> > + *		Group 1: 5 - 13
> > + *		Group 2: 0 - 7
> > + *		Group 3: 0 - 5
> > + *		Group 4: 0 - 1
> > + */
> > +struct dwc_pcie_event_counter {
> > +	const char *name;
> > +	u32 group_no;
> > +	u32 event_no;
> > +};
> > +
> > +static const struct dwc_pcie_event_counter event_list[] = {
> > +	{"ebuf_overflow", 0x0, 0x0},
> > +	{"ebuf_underrun", 0x0, 0x1},
> > +	{"decode_err", 0x0, 0x2},
> > +	{"running_disparity_err", 0x0, 0x3},
> > +	{"skp_os_parity_err", 0x0, 0x4},
> > +	{"sync_header_err", 0x0, 0x5},
> > +	{"rx_valid_deassertion", 0x0, 0x6},
> > +	{"ctl_skp_os_parity_err", 0x0, 0x7},
> > +	{"retimer_parity_err_1st", 0x0, 0x8},
> > +	{"retimer_parity_err_2nd", 0x0, 0x9},
> > +	{"margin_crc_parity_err", 0x0, 0xA},
> > +	{"detect_ei_infer", 0x1, 0x5},
> > +	{"receiver_err", 0x1, 0x6},
> > +	{"rx_recovery_req", 0x1, 0x7},
> > +	{"n_fts_timeout", 0x1, 0x8},
> > +	{"framing_err", 0x1, 0x9},
> > +	{"deskew_err", 0x1, 0xa},
> > +	{"framing_err_in_l0", 0x1, 0xc},
> > +	{"deskew_uncompleted_err", 0x1, 0xd},
> > +	{"bad_tlp", 0x2, 0x0},
> > +	{"lcrc_err", 0x2, 0x1},
> > +	{"bad_dllp", 0x2, 0x2},
> > +	{"replay_num_rollover", 0x2, 0x3},
> > +	{"replay_timeout", 0x2, 0x4},
> > +	{"rx_nak_dllp", 0x2, 0x5},
> > +	{"tx_nak_dllp", 0x2, 0x6},
> > +	{"retry_tlp", 0x2, 0x7},
> > +	{"fc_timeout", 0x3, 0x0},
> > +	{"poisoned_tlp", 0x3, 0x1},
> > +	{"ecrc_error", 0x3, 0x2},
> > +	{"unsupported_request", 0x3, 0x3},
> > +	{"completer_abort", 0x3, 0x4},
> > +	{"completion_timeout", 0x3, 0x5},
> > +	{"ebuf_skp_add", 0x4, 0x0},
> > +	{"ebuf_skp_del", 0x4, 0x1},
> > +};
> > +
> > +/**
> > + * struct dwc_pcie_err_inj - Store details about each error injection
> > +supported by DWC RASDES
> > + * @name: Name of the error that can be injected
> > + * @err_inj_group: Group number to which the error belongs to. Value
> > +can range from 0 - 5
> > + * @err_inj_type: Each group can have multiple types of error  */
> > +struct dwc_pcie_err_inj {
> > +	const char *name;
> > +	u32 err_inj_group;
> > +	u32 err_inj_type;
> > +};
> > +
> > +static const struct dwc_pcie_err_inj err_inj_list[] = {
> > +	{"tx_lcrc", 0x0, 0x0},
> > +	{"b16_crc_dllp", 0x0, 0x1},
> > +	{"b16_crc_upd_fc", 0x0, 0x2},
> > +	{"tx_ecrc", 0x0, 0x3},
> > +	{"fcrc_tlp", 0x0, 0x4},
> > +	{"parity_tsos", 0x0, 0x5},
> > +	{"parity_skpos", 0x0, 0x6},
> > +	{"rx_lcrc", 0x0, 0x8},
> > +	{"rx_ecrc", 0x0, 0xb},
> > +	{"tlp_err_seq", 0x1, 0x0},
> > +	{"ack_nak_dllp_seq", 0x1, 0x1},
> > +	{"ack_nak_dllp", 0x2, 0x0},
> > +	{"upd_fc_dllp", 0x2, 0x1},
> > +	{"nak_dllp", 0x2, 0x2},
> > +	{"inv_sync_hdr_sym", 0x3, 0x0},
> > +	{"com_pad_ts1", 0x3, 0x1},
> > +	{"com_pad_ts2", 0x3, 0x2},
> > +	{"com_fts", 0x3, 0x3},
> > +	{"com_idl", 0x3, 0x4},
> > +	{"end_edb", 0x3, 0x5},
> > +	{"stp_sdp", 0x3, 0x6},
> > +	{"com_skp", 0x3, 0x7},
> > +	{"posted_tlp_hdr", 0x4, 0x0},
> > +	{"non_post_tlp_hdr", 0x4, 0x1},
> > +	{"cmpl_tlp_hdr", 0x4, 0x2},
> > +	{"posted_tlp_data", 0x4, 0x4},
> > +	{"non_post_tlp_data", 0x4, 0x5},
> > +	{"cmpl_tlp_data", 0x4, 0x6},
> > +	{"duplicate_dllp", 0x5, 0x0},
> > +	{"nullified_tlp", 0x5, 0x1},
> > +};
> > +
> > +static const u32 err_inj_type_mask[] = {
> > +	EINJ0_TYPE,
> > +	EINJ1_TYPE,
> > +	EINJ2_TYPE,
> > +	EINJ3_TYPE,
> > +	EINJ4_TYPE,
> > +	EINJ5_TYPE,
> > +};
> > +
> > +static ssize_t lane_detect_read(struct file *file, char __user *buf,
> > +size_t count, loff_t *ppos) {
> > +	struct dw_pcie *pci = file->private_data;
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > +	ssize_t off = 0;
> > +	u32 val;
> > +
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
> > +	val = FIELD_GET(PIPE_DETECT_LANE, val);
> > +	if (val)
> > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane Detected\n");
> > +	else
> > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane
> > +Undetected\n");
> > +
> > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> > +}
> > +
> > +static ssize_t lane_detect_write(struct file *file, const char __user *buf,
> > +				 size_t count, loff_t *ppos)
> > +{
> > +	struct dw_pcie *pci = file->private_data;
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +	u32 lane, val;
> > +
> > +	val = kstrtou32_from_user(buf, count, 0, &lane);
> > +	if (val)
> > +		return val;
> > +
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
> > +	val &= ~(LANE_SELECT);
> > +	val |= FIELD_PREP(LANE_SELECT, lane);
> > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset +
> > +SD_STATUS_L1LANE_REG, val);
> > +
> > +	return count;
> > +}
> > +
> > +static ssize_t rx_valid_read(struct file *file, char __user *buf,
> > +size_t count, loff_t *ppos) {
> > +	struct dw_pcie *pci = file->private_data;
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > +	ssize_t off = 0;
> > +	u32 val;
> > +
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
> > +	val = FIELD_GET(PIPE_RXVALID, val);
> > +	if (val)
> > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "RX Valid\n");
> > +	else
> > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "RX
> > +Invalid\n");
> > +
> > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> > +}
> > +
> > +static ssize_t rx_valid_write(struct file *file, const char __user
> > +*buf, size_t count, loff_t *ppos) {
> > +	return lane_detect_write(file, buf, count, ppos); }
> > +
> > +static void set_event_number(struct dwc_pcie_rasdes_priv *pdata, struct dw_pcie *pci,
> > +			     struct dwc_pcie_rasdes_info *rinfo) {
> > +	u32 val;
> > +
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > +	val &= ~EVENT_COUNTER_ENABLE;
> > +	val &= ~(EVENT_COUNTER_GROUP_SELECT | EVENT_COUNTER_EVENT_SELECT);
> > +	val |= FIELD_PREP(EVENT_COUNTER_GROUP_SELECT, event_list[pdata->idx].group_no);
> > +	val |= FIELD_PREP(EVENT_COUNTER_EVENT_SELECT, event_list[pdata->idx].event_no);
> > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset +
> > +RAS_DES_EVENT_COUNTER_CTRL_REG, val); }
> > +
> > +static ssize_t counter_enable_read(struct file *file, char __user
> > +*buf, size_t count, loff_t *ppos) {
> > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > +	struct dw_pcie *pci = pdata->pci;
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > +	ssize_t off = 0;
> > +	u32 val;
> > +
> > +	mutex_lock(&rinfo->reg_lock);
> > +	set_event_number(pdata, pci, rinfo);
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > +	mutex_unlock(&rinfo->reg_lock);
> > +	val = FIELD_GET(EVENT_COUNTER_STATUS, val);
> > +	if (val)
> > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter Enabled\n");
> > +	else
> > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter
> > +Disabled\n");
> > +
> > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> > +}
> > +
> > +static ssize_t counter_enable_write(struct file *file, const char __user *buf,
> > +				    size_t count, loff_t *ppos)
> > +{
> > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > +	struct dw_pcie *pci = pdata->pci;
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +	u32 val, enable;
> > +
> > +	val = kstrtou32_from_user(buf, count, 0, &enable);
> > +	if (val)
> > +		return val;
> > +
> > +	mutex_lock(&rinfo->reg_lock);
> > +	set_event_number(pdata, pci, rinfo);
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > +	if (enable)
> > +		val |= FIELD_PREP(EVENT_COUNTER_ENABLE, PER_EVENT_ON);
> > +	else
> > +		val |= FIELD_PREP(EVENT_COUNTER_ENABLE, PER_EVENT_OFF);
> > +
> > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
> > +	mutex_unlock(&rinfo->reg_lock);
> > +
> > +	return count;
> > +}
> > +
> > +static ssize_t counter_lane_read(struct file *file, char __user *buf,
> > +size_t count, loff_t *ppos) {
> > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > +	struct dw_pcie *pci = pdata->pci;
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > +	ssize_t off = 0;
> > +	u32 val;
> > +
> > +	mutex_lock(&rinfo->reg_lock);
> > +	set_event_number(pdata, pci, rinfo);
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > +	mutex_unlock(&rinfo->reg_lock);
> > +	val = FIELD_GET(EVENT_COUNTER_LANE_SELECT, val);
> > +	off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane:
> > +%d\n", val);
> > +
> > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> > +}
> > +
> > +static ssize_t counter_lane_write(struct file *file, const char __user *buf,
> > +				  size_t count, loff_t *ppos)
> > +{
> > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > +	struct dw_pcie *pci = pdata->pci;
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +	u32 val, lane;
> > +
> > +	val = kstrtou32_from_user(buf, count, 0, &lane);
> > +	if (val)
> > +		return val;
> > +
> > +	mutex_lock(&rinfo->reg_lock);
> > +	set_event_number(pdata, pci, rinfo);
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > +	val &= ~(EVENT_COUNTER_LANE_SELECT);
> > +	val |= FIELD_PREP(EVENT_COUNTER_LANE_SELECT, lane);
> > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
> > +	mutex_unlock(&rinfo->reg_lock);
> > +
> > +	return count;
> > +}
> > +
> > +static ssize_t counter_value_read(struct file *file, char __user
> > +*buf, size_t count, loff_t *ppos) {
> > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > +	struct dw_pcie *pci = pdata->pci;
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > +	ssize_t off = 0;
> > +	u32 val;
> > +
> > +	mutex_lock(&rinfo->reg_lock);
> > +	set_event_number(pdata, pci, rinfo);
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_DATA_REG);
> > +	mutex_unlock(&rinfo->reg_lock);
> > +	off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter
> > +value: %d\n", val);
> > +
> > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf, off);
> > +}
> > +
> > +static ssize_t err_inj_write(struct file *file, const char __user
> > +*buf, size_t count, loff_t *ppos) {
> > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > +	struct dw_pcie *pci = pdata->pci;
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +	u32 val, counter, vc_num, err_group, type_mask;
> > +	int val_diff = 0;
> > +	char *kern_buf;
> > +
> > +	err_group = err_inj_list[pdata->idx].err_inj_group;
> > +	type_mask = err_inj_type_mask[err_group];
> > +
> > +	kern_buf = memdup_user_nul(buf, count);
> > +	if (IS_ERR(kern_buf))
> > +		return PTR_ERR(kern_buf);
> > +
> > +	if (err_group == 4) {
> > +		val = sscanf(kern_buf, "%u %d %u", &counter, &val_diff, &vc_num);
> > +		if ((val != 3) || (val_diff < -4095 || val_diff > 4095)) {
> > +			kfree(kern_buf);
> > +			return -EINVAL;
> > +		}
> > +	} else if (err_group == 1) {
> > +		val = sscanf(kern_buf, "%u %d", &counter, &val_diff);
> > +		if ((val != 2) || (val_diff < -4095 || val_diff > 4095)) {
> > +			kfree(kern_buf);
> > +			return -EINVAL;
> > +		}
> > +	} else {
> > +		val = kstrtou32(kern_buf, 0, &counter);
> > +		if (val) {
> > +			kfree(kern_buf);
> > +			return val;
> > +		}
> > +	}
> > +
> > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + ERR_INJ0_OFF + (0x4 * err_group));
> > +	val &= ~(type_mask | EINJ_COUNT);
> > +	val |= ((err_inj_list[pdata->idx].err_inj_type << EINJ_TYPE_SHIFT) & type_mask);
> > +	val |= FIELD_PREP(EINJ_COUNT, counter);
> > +
> > +	if (err_group == 1 || err_group == 4) {
> > +		val &= ~(EINJ_VAL_DIFF);
> > +		val |= FIELD_PREP(EINJ_VAL_DIFF, val_diff);
> > +	}
> > +	if (err_group == 4) {
> > +		val &= ~(EINJ_VC_NUM);
> > +		val |= FIELD_PREP(EINJ_VC_NUM, vc_num);
> > +	}
> > +
> > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + ERR_INJ0_OFF + (0x4 * err_group), val);
> > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + ERR_INJ_ENABLE_REG,
> > +(0x1 << err_group));
> > +
> > +	kfree(kern_buf);
> > +	return count;
> > +}
> > +
> > +#define dwc_debugfs_create(name)			\
> > +debugfs_create_file(#name, 0644, rasdes_debug, pci,	\
> > +			&dbg_ ## name ## _fops)
> > +
> > +#define DWC_DEBUGFS_FOPS(name)					\
> > +static const struct file_operations dbg_ ## name ## _fops = {	\
> > +	.open = simple_open,				\
> > +	.read = name ## _read,				\
> > +	.write = name ## _write				\
> > +}
> > +
> > +DWC_DEBUGFS_FOPS(lane_detect);
> > +DWC_DEBUGFS_FOPS(rx_valid);
> > +
> > +static const struct file_operations dwc_pcie_counter_enable_ops = {
> > +	.open = simple_open,
> > +	.read = counter_enable_read,
> > +	.write = counter_enable_write,
> > +};
> > +
> > +static const struct file_operations dwc_pcie_counter_lane_ops = {
> > +	.open = simple_open,
> > +	.read = counter_lane_read,
> > +	.write = counter_lane_write,
> > +};
> > +
> > +static const struct file_operations dwc_pcie_counter_value_ops = {
> > +	.open = simple_open,
> > +	.read = counter_value_read,
> > +};
> > +
> > +static const struct file_operations dwc_pcie_err_inj_ops = {
> > +	.open = simple_open,
> > +	.write = err_inj_write,
> > +};
> > +
> > +void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci) {
> > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > +
> > +	debugfs_remove_recursive(rinfo->rasdes_dir);
> > +	mutex_destroy(&rinfo->reg_lock);
> > +}
> > +
> > +int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci) {
> > +	struct dentry *dir, *rasdes_debug, *rasdes_err_inj, *rasdes_event_counter, *rasdes_events;
> > +	struct dwc_pcie_rasdes_info *rasdes_info;
> > +	struct dwc_pcie_rasdes_priv *priv_tmp;
> > +	char dirname[DWC_DEBUGFS_BUF_MAX];
> > +	struct device *dev = pci->dev;
> > +	int ras_cap, i, ret;
> > +
> > +	ras_cap = dw_pcie_find_vsec_capability(pci, DW_PCIE_VSEC_EXT_CAP_RAS_DES);
> > +	if (!ras_cap) {
> > +		dev_dbg(dev, "No RASDES capability available\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	rasdes_info = devm_kzalloc(dev, sizeof(*rasdes_info), GFP_KERNEL);
> > +	if (!rasdes_info)
> > +		return -ENOMEM;
> > +
> > +	/* Create main directory for each platform driver */
> > +	snprintf(dirname, DWC_DEBUGFS_BUF_MAX, "dwc_pcie_%s", dev_name(dev));
> > +	dir = debugfs_create_dir(dirname, NULL);
> > +	if (IS_ERR(dir))
> > +		return PTR_ERR(dir);
> > +
> > +	/* Create subdirectories for Debug, Error injection, Statistics */
> > +	rasdes_debug = debugfs_create_dir("rasdes_debug", dir);
> > +	rasdes_err_inj = debugfs_create_dir("rasdes_err_inj", dir);
> > +	rasdes_event_counter = debugfs_create_dir("rasdes_event_counter",
> > +dir);
> > +
> > +	mutex_init(&rasdes_info->reg_lock);
> > +	rasdes_info->ras_cap_offset = ras_cap;
> > +	rasdes_info->rasdes_dir = dir;
> > +	pci->rasdes_info = rasdes_info;
> > +
> > +	/* Create debugfs files for Debug subdirectory */
> > +	dwc_debugfs_create(lane_detect);
> > +	dwc_debugfs_create(rx_valid);
> > +
> > +	/* Create debugfs files for Error injection subdirectory */
> > +	for (i = 0; i < ARRAY_SIZE(err_inj_list); i++) {
> > +		priv_tmp = devm_kzalloc(dev, sizeof(*priv_tmp), GFP_KERNEL);
> > +		if (!priv_tmp) {
> > +			ret = -ENOMEM;
> > +			goto err_deinit;
> > +		}
> > +
> > +		priv_tmp->idx = i;
> > +		priv_tmp->pci = pci;
> > +		debugfs_create_file(err_inj_list[i].name, 0200, rasdes_err_inj, priv_tmp,
> > +				    &dwc_pcie_err_inj_ops);
> > +	}
> > +
> > +	/* Create debugfs files for Statistical counter subdirectory */
> > +	for (i = 0; i < ARRAY_SIZE(event_list); i++) {
> > +		priv_tmp = devm_kzalloc(dev, sizeof(*priv_tmp), GFP_KERNEL);
> > +		if (!priv_tmp) {
> > +			ret = -ENOMEM;
> > +			goto err_deinit;
> > +		}
> > +
> > +		priv_tmp->idx = i;
> > +		priv_tmp->pci = pci;
> > +		rasdes_events = debugfs_create_dir(event_list[i].name, rasdes_event_counter);
> > +		if (event_list[i].group_no == 0 || event_list[i].group_no == 4) {
> > +			debugfs_create_file("lane_select", 0644, rasdes_events,
> > +					    priv_tmp, &dwc_pcie_counter_lane_ops);
> > +		}
> > +		debugfs_create_file("counter_value", 0444, rasdes_events, priv_tmp,
> > +				    &dwc_pcie_counter_value_ops);
> > +		debugfs_create_file("counter_enable", 0644, rasdes_events, priv_tmp,
> > +				    &dwc_pcie_counter_enable_ops);
> > +	}
> > +
> > +	return 0;
> > +
> > +err_deinit:
> > +	dwc_pcie_rasdes_debugfs_deinit(pci);
> > +	return ret;
> > +}
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.h
> > b/drivers/pci/controller/dwc/pcie-designware-debugfs.h
> > new file mode 100644
> > index 000000000000..e69de29bb2d1
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index 98a057820bc7..ed0f26d69626 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -260,6 +260,8 @@
> >
> >  #define PCIE_RAS_DES_EVENT_COUNTER_DATA		0xc
> >
> > +#define DW_PCIE_VSEC_EXT_CAP_RAS_DES		0x2
> > +
> >  /*
> >   * The default address offset between dbi_base and atu_base. Root controller
> >   * drivers are not required to initialize atu_base if the offset
> > matches this @@ -463,6 +465,7 @@ struct dw_pcie {
> >  	struct reset_control_bulk_data	core_rsts[DW_PCIE_NUM_CORE_RSTS];
> >  	struct gpio_desc		*pe_rst;
> >  	bool			suspended;
> > +	void			*rasdes_info;
> >  };
> >
> >  #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie,
> > pp) @@ -796,4 +799,18 @@ dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
> >  	return NULL;
> >  }
> >  #endif
> > +
> > +#ifdef CONFIG_PCIE_DW_DEBUGFS
> > +int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci); void
> > +dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci); #else static
> > +inline int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci) {
> > +	return 0;
> > +}
> > +static inline void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie
> > +*pci) { } #endif
> > +
> >  #endif /* _PCIE_DESIGNWARE_H */
> > --
> > 2.17.1
> >
> 
> --
> Fan Ni (From gmail)

RE: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Shradha Todi]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 06 December 2024 21:43
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> fan.ni@samsung.com; a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> quic_krichai@quicinc.com; gost.dev@samsung.com
> Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> 
> On Fri, Dec 06, 2024 at 01:14:55PM +0530, Shradha Todi wrote:
> > Add vendor specific extended configuration space capability search API
> > using struct dw_pcie pointer for DW controllers.
> >
> > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware.c | 16 ++++++++++++++++
> > drivers/pci/controller/dwc/pcie-designware.h |  1 +
> >  2 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > b/drivers/pci/controller/dwc/pcie-designware.c
> > index 6d6cbc8b5b2c..41230c5e4a53 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> >  	return 0;
> >  }
> >
> > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
> 
> To make sure that we find a VSEC ID that corresponds to the expected vendor, I think this interface needs to be the
same
> as pci_find_vsec_capability().  In particular, it needs to take a "u16 vendor"

As per my understanding, Synopsys is the vendor here when we talk about vsec capabilities.
VSEC cap IDs are fixed for each vendor (eg: For Synopsys Designware controllers, 0x2 is always RAS CAP,
0x4 is always PTM responder and so on).
So no matter if the DWC IP is being integrated by Samsung, NVDIA or Qcom, the vendor specific CAP IDs will
remain constant. Now since this function is being written as part of designware file, the control will reach here
only when the PCIe IP is DWC. So, we don't really require a vendor ID to be checked here. EG: If 0x2 VSEC ID is present
in any DWC controller, it means RAS is supported. Please correct me if I'm wrong.

>and a "u16 vsec_cap".
> 
> (pci_find_vsec_capability() takes an "int cap", but I don't think that's quite right).
> 

It should be u16 vsec_cap. You're right. I will fix this in the next patchset.

Shradha

> > +{
> > +	u16 vsec = 0;
> > +	u32 header;
> > +
> > +	while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
> > +					PCI_EXT_CAP_ID_VNDR)) {
> > +		header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
> > +		if (PCI_VNDR_HEADER_ID(header) == vsec_cap)
> > +			return vsec;
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(dw_pcie_find_vsec_capability);
> > +
> >  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap)  {
> >  	return dw_pcie_find_next_ext_capability(pci, 0, cap); diff --git
> > a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index 347ab74ac35a..98a057820bc7 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -476,6 +476,7 @@ void dw_pcie_version_detect(struct dw_pcie *pci);
> >
> >  u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
> >  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap);
> > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap);
> >
> >  int dw_pcie_read(void __iomem *addr, int size, u32 *val);  int
> > dw_pcie_write(void __iomem *addr, int size, u32 val);
> > --
> > 2.17.1
> >

Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Bjorn Helgaas]

On Wed, Dec 11, 2024 at 05:15:50PM +0530, Shradha Todi wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: 06 December 2024 21:43
> > To: Shradha Todi <shradha.t@samsung.com>
> > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> > kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> > fan.ni@samsung.com; a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> > quic_krichai@quicinc.com; gost.dev@samsung.com
> > Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> > 
> > On Fri, Dec 06, 2024 at 01:14:55PM +0530, Shradha Todi wrote:
> > > Add vendor specific extended configuration space capability search API
> > > using struct dw_pcie pointer for DW controllers.
> > >
> > > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-designware.c | 16 ++++++++++++++++
> > > drivers/pci/controller/dwc/pcie-designware.h |  1 +
> > >  2 files changed, 17 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 6d6cbc8b5b2c..41230c5e4a53 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> > >  	return 0;
> > >  }
> > >
> > > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
> > 
> > To make sure that we find a VSEC ID that corresponds to the
> > expected vendor, I think this interface needs to be the same
> > as pci_find_vsec_capability().  In particular, it needs to take a
> > "u16 vendor"
>
> As per my understanding, Synopsys is the vendor here when we talk
> about vsec capabilities.  VSEC cap IDs are fixed for each vendor
> (eg: For Synopsys Designware controllers, 0x2 is always RAS CAP, 0x4
> is always PTM responder and so on).

For VSEC, the vendor that matters is the one identified at 0x0 in
config space.  That's why pci_find_vsec_capability() checks the
supplied "vendor" against "dev->vendor".

> So no matter if the DWC IP is being integrated by Samsung, NVDIA or
> Qcom, the vendor specific CAP IDs will remain constant. Now since
> this function is being written as part of designware file, the
> control will reach here only when the PCIe IP is DWC. So, we don't
> really require a vendor ID to be checked here. EG: If 0x2 VSEC ID is
> present in any DWC controller, it means RAS is supported. Please
> correct me if I'm wrong.

In this case, the Vendor ID is typically Samsung, NVIDIA, Qcom, etc.,
even though it may contain Synopsys DWC IP.  Each vendor assigns VSEC
IDs independently, so VSEC ID 0x2 may mean something different to
Samsung than it does to NVIDIA or Qcom.

PCIe r6.0, sec 7.9.5 has the details, but the important part is this:

  With a PCI Express Function, the structure and definition of the
  vendor-specific Registers area is determined by the vendor indicated
  by the Vendor ID field located at byte offset 00h in PCI-compatible
  Configuration Space.

There IS a separate DVSEC ("Designated Vendor-Specific") Capability;
see sec 7.9.6.  That one does include a DVSEC Vendor ID in the
Capability itself, and this would make more sense for this situation.

If Synopsys assigned DVSEC ID 0x2 from the Synopsys namespace for RAS,
then devices from Samsung, NVIDIA, Qcom, etc., could advertise a DVSEC
Capability that contained a DVSEC Vendor ID of PCI_VENDOR_ID_SYNOPSYS
with DVSEC ID 0x2, and all those devices could easily locate it.

Unfortunately Samsung et al used VSEC instead of DVSEC, so we're stuck
with having to specify the device vendor and the VSEC ID assigned by
that vendor, and those VSEC IDs might be different per vendor.

Bjorn

RE: [PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support in DWC

[Shradha Todi]

Hi all,

Gentle reminder to review this patch series.

> -----Original Message-----
> From: Shradha Todi <shradha.t@samsung.com>
> Sent: 11 December 2024 16:39
> To: 'Fan Ni' <nifan.cxl@gmail.com>
> Cc: 'linux-kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>; 'linux-pci@vger.kernel.org' <linux-
> pci@vger.kernel.org>; 'manivannan.sadhasivam@linaro.org' <manivannan.sadhasivam@linaro.org>; 'lpieralisi@kernel.org'
> <lpieralisi@kernel.org>; 'kw@linux.com' <kw@linux.com>; 'robh@kernel.org' <robh@kernel.org>; 'bhelgaas@google.com'
> <bhelgaas@google.com>; 'jingoohan1@gmail.com' <jingoohan1@gmail.com>; 'Jonathan.Cameron@huawei.com'
> <Jonathan.Cameron@huawei.com>; 'a.manzanares@samsung.com' <a.manzanares@samsung.com>;
> 'pankaj.dubey@samsung.com' <pankaj.dubey@samsung.com>; 'quic_nitegupt@quicinc.com'
> <quic_nitegupt@quicinc.com>; 'quic_krichai@quicinc.com' <quic_krichai@quicinc.com>; 'gost.dev@samsung.com'
> <gost.dev@samsung.com>
> Subject: RE: [PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support in DWC
> 
> 
> 
> > -----Original Message-----
> > From: Fan Ni <nifan.cxl@gmail.com>
> > Sent: 10 December 2024 03:59
> > To: Shradha Todi <shradha.t@samsung.com>
> > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org;
> > manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org; kw@linux.com;
> > robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com;
> > Jonathan.Cameron@huawei.com; a.manzanares@samsung.com;
> > pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> > quic_krichai@quicinc.com; gost.dev@samsung.com
> > Subject: Re: [PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support
> > in DWC
> >
> > On Fri, Dec 06, 2024 at 01:14:56PM +0530, Shradha Todi wrote:
> > > Add support to use the RASDES feature of DesignWare PCIe controller
> > > using debugfs entries.
> > >
> > > RASDES is a vendor specific extended PCIe capability which reads the
> > > current hardware internal state of PCIe device. Following primary
> > > features are provided to userspace via debugfs:
> > > - Debug registers
> > > - Error injection
> > > - Statistical counters
> >
> > I think this patch can break into several to make it easier to review.
> > For example, it can be divided by the three features list above, with
> > the documentation change coming last as a separate patch.
> >
> 
> Sure Fan. I have no issues in breaking this into smaller patches. Though I think the documentation should go along
with the
> implementation rather than a separate patch?
> Anyway, I'll wait for some time for further review comments or if anyone has any objection to splitting the patches
before
> going for the next revision.
> 
> > Some minor comments inline.
> >
> > >
> > > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > > ---
> > >  Documentation/ABI/testing/debugfs-dwc-pcie    | 143 +++++
> > >  drivers/pci/controller/dwc/Kconfig            |  11 +
> > >  drivers/pci/controller/dwc/Makefile           |   1 +
> > >  .../controller/dwc/pcie-designware-debugfs.c  | 544 ++++++++++++++++++
> > >  .../controller/dwc/pcie-designware-debugfs.h  |   0
> > >  drivers/pci/controller/dwc/pcie-designware.h  |  17 +
> > >  6 files changed, 716 insertions(+)
> > >  create mode 100644 Documentation/ABI/testing/debugfs-dwc-pcie
> > >  create mode 100644
> > > drivers/pci/controller/dwc/pcie-designware-debugfs.c
> > >  create mode 100644
> > > drivers/pci/controller/dwc/pcie-designware-debugfs.h
> > >
> > > diff --git a/Documentation/ABI/testing/debugfs-dwc-pcie
> > > b/Documentation/ABI/testing/debugfs-dwc-pcie
> > > new file mode 100644
> > > index 000000000000..7da73ac8d40c
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/debugfs-dwc-pcie
> > > @@ -0,0 +1,143 @@
> > > +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/lane_detect
> > > +Date:		December 2024
> > > +Contact:	Shradha Todi <shradha.t@samsung.com>
> > > +Description:	(RW) Write the lane number to be checked for detection.	Read
> > > +		will dump whether PHY indicates receiver detection on the
> > > +		selected lane.
> > > +
> > > +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/rx_valid
> > > +Date:		December 2024
> > > +Contact:	Shradha Todi <shradha.t@samsung.com>
> > > +Description:	(RW) Write the lane number to be checked as valid or invalid. Read
> > > +		will dump the status of PIPE RXVALID signal of the selected lane.
> > > +
> > > +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_event_counters/<event>/counter_enable
> >
> > I think "counter_enable" can be "enable" since the upper directory already shows it is for event counter.
> >
> 
> Makes sense. I'll fix this in the next patchset.
> 
> > > +Date:		December 2024
> > > +Contact:	Shradha Todi <shradha.t@samsung.com>
> > > +Description:	rasdes_event_counters is the directory which can be used to collect
> > > +		statistical data about the number of times a certain event has occurred
> > > +		in the controller. The list of possible events are:
> > > +
> > > +		1) EBUF Overflow
> > > +		2) EBUF Underrun
> > > +		3) Decode Error
> > > +		4) Running Disparity Error
> > > +		5) SKP OS Parity Error
> > > +		6) SYNC Header Error
> > > +		7) Rx Valid De-assertion
> > > +		8) CTL SKP OS Parity Error
> > > +		9) 1st Retimer Parity Error
> > > +		10) 2nd Retimer Parity Error
> > > +		11) Margin CRC and Parity Error
> > > +		12) Detect EI Infer
> > > +		13) Receiver Error
> > > +		14) RX Recovery Req
> > > +		15) N_FTS Timeout
> > > +		16) Framing Error
> > > +		17) Deskew Error
> > > +		18) Framing Error In L0
> > > +		19) Deskew Uncompleted Error
> > > +		20) Bad TLP
> > > +		21) LCRC Error
> > > +		22) Bad DLLP
> > > +		23) Replay Number Rollover
> > > +		24) Replay Timeout
> > > +		25) Rx Nak DLLP
> > > +		26) Tx Nak DLLP
> > > +		27) Retry TLP
> > > +		28) FC Timeout
> > > +		29) Poisoned TLP
> > > +		30) ECRC Error
> > > +		31) Unsupported Request
> > > +		32) Completer Abort
> > > +		33) Completion Timeout
> > > +		34) EBUF SKP Add
> > > +		35) EBUF SKP Del
> > > +
> > > +		counter_enable is RW. Write 1 to enable the event counter and write 0 to
> > > +		disable the event counter. Read will dump whether the counter is currently
> > > +		enabled	or disabled.
> > ...
> > > diff --git a/drivers/pci/controller/dwc/Kconfig
> > > b/drivers/pci/controller/dwc/Kconfig
> > > index b6d6778b0698..9ab8d724fe0d 100644
> > > --- a/drivers/pci/controller/dwc/Kconfig
> > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > @@ -6,6 +6,17 @@ menu "DesignWare-based PCIe controllers"
> > >  config PCIE_DW
> > >  	bool
> > >
> > > +config PCIE_DW_DEBUGFS
> > > +	default y
> > Why we need to enable it by default?
> >
> 
> Actually the config depends on debug_fs, so the intention was that if debug_fs and atleast one DW driver is enabled in
the
> kernel, there should be no harm in enabling this.
> 
> > Fan
> > > +	depends on DEBUG_FS
> > > +	depends on PCIE_DW_HOST || PCIE_DW_EP
> > > +	bool "DWC PCIe debugfs entries"
> > > +	help
> > > +	  Enables debugfs entries for the DWC PCIe Controller.
> > > +	  These entries make use of the RAS features in the DW
> > > +	  controller to help in debug, error injection and statistical
> > > +	  counters
> > > +
> > >  config PCIE_DW_HOST
> > >  	bool
> > >  	select PCIE_DW
> > > diff --git a/drivers/pci/controller/dwc/Makefile
> > > b/drivers/pci/controller/dwc/Makefile
> > > index a8308d9ea986..54565eedc52c 100644
> > > --- a/drivers/pci/controller/dwc/Makefile
> > > +++ b/drivers/pci/controller/dwc/Makefile
> > > @@ -1,5 +1,6 @@
> > >  # SPDX-License-Identifier: GPL-2.0
> > >  obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> > > +obj-$(CONFIG_PCIE_DW_DEBUGFS) += pcie-designware-debugfs.o
> > >  obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
> > >  obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> > >  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o diff --git
> > > a/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> > > b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> > > new file mode 100644
> > > index 000000000000..a93e29993f75
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> > > @@ -0,0 +1,544 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Synopsys DesignWare PCIe controller debugfs driver
> > > + *
> > > + * Copyright (C) 2024 Samsung Electronics Co., Ltd.
> > > + *		http://www.samsung.com
> > > + *
> > > + * Author: Shradha Todi <shradha.t@samsung.com>  */
> > > +
> > > +#include <linux/debugfs.h>
> > > +
> > > +#include "pcie-designware.h"
> > > +
> > > +#define SD_STATUS_L1LANE_REG		0xb0
> > > +#define PIPE_DETECT_LANE		BIT(17)
> > > +#define PIPE_RXVALID			BIT(18)
> > > +#define LANE_SELECT			GENMASK(3, 0)
> > > +
> > > +#define RAS_DES_EVENT_COUNTER_CTRL_REG	0x8
> > > +#define EVENT_COUNTER_GROUP_SELECT	GENMASK(27, 24)
> > > +#define EVENT_COUNTER_EVENT_SELECT	GENMASK(23, 16)
> > > +#define EVENT_COUNTER_LANE_SELECT	GENMASK(11, 8)
> > > +#define EVENT_COUNTER_STATUS		BIT(7)
> > > +#define EVENT_COUNTER_ENABLE		GENMASK(4, 2)
> > > +#define PER_EVENT_OFF			0x1
> > > +#define PER_EVENT_ON			0x3
> > > +
> > > +#define RAS_DES_EVENT_COUNTER_DATA_REG	0xc
> > > +
> > > +#define ERR_INJ_ENABLE_REG		0x30
> > > +#define ERR_INJ0_OFF			0x34
> > > +#define EINJ_COUNT			GENMASK(7, 0)
> > > +#define EINJ_TYPE_SHIFT			8
> > > +#define EINJ0_TYPE			GENMASK(11, 8)
> > > +#define EINJ1_TYPE			BIT(8)
> > > +#define EINJ2_TYPE			GENMASK(9, 8)
> > > +#define EINJ3_TYPE			GENMASK(10, 8)
> > > +#define EINJ4_TYPE			GENMASK(10, 8)
> > > +#define EINJ5_TYPE			BIT(8)
> > > +#define EINJ_VC_NUM			GENMASK(14, 12)
> > > +#define EINJ_VAL_DIFF			GENMASK(28, 16)
> > > +
> > > +#define DWC_DEBUGFS_BUF_MAX		128
> > > +
> > > +/**
> > > + * struct dwc_pcie_rasdes_info - Stores controller common
> > > +information
> > > + * @ras_cap_offset: RAS DES vendor specific extended capability
> > > +offset
> > > + * @reg_lock: Mutex used for RASDES shadow event registers
> > > + * @rasdes_dir: Top level debugfs directory entry
> > > + *
> > > + * Any parameter constant to all files of the debugfs hierarchy for
> > > +a single controller
> > > + * will be stored in this struct. It is allocated and assigned to
> > > +controller specific
> > > + * struct dw_pcie during initialization.
> > > + */
> > > +struct dwc_pcie_rasdes_info {
> > > +	u32 ras_cap_offset;
> > > +	struct mutex reg_lock;
> > > +	struct dentry *rasdes_dir;
> > > +};
> > > +
> > > +/**
> > > + * struct dwc_pcie_rasdes_priv - Stores file specific private data
> > > +information
> > > + * @pci: Reference to the dw_pcie structure
> > > + * @idx: Index to point to specific file related information in
> > > +array of structs
> > > + *
> > > + * All debugfs files will have this struct as its private data.
> > > + */
> > > +struct dwc_pcie_rasdes_priv {
> > > +	struct dw_pcie *pci;
> > > +	int idx;
> > > +};
> > > +
> > > +/**
> > > + * struct dwc_pcie_event_counter - Store details about each event
> > > +counter supported in DWC RASDES
> > > + * @name: Name of the error counter
> > > + * @group_no: Group number that the event belongs to. Value ranges
> > > +from 0 - 4
> > > + * @event_no: Event number of the particular event. Value ranges from -
> > > + *		Group 0: 0 - 10
> > > + *		Group 1: 5 - 13
> > > + *		Group 2: 0 - 7
> > > + *		Group 3: 0 - 5
> > > + *		Group 4: 0 - 1
> > > + */
> > > +struct dwc_pcie_event_counter {
> > > +	const char *name;
> > > +	u32 group_no;
> > > +	u32 event_no;
> > > +};
> > > +
> > > +static const struct dwc_pcie_event_counter event_list[] = {
> > > +	{"ebuf_overflow", 0x0, 0x0},
> > > +	{"ebuf_underrun", 0x0, 0x1},
> > > +	{"decode_err", 0x0, 0x2},
> > > +	{"running_disparity_err", 0x0, 0x3},
> > > +	{"skp_os_parity_err", 0x0, 0x4},
> > > +	{"sync_header_err", 0x0, 0x5},
> > > +	{"rx_valid_deassertion", 0x0, 0x6},
> > > +	{"ctl_skp_os_parity_err", 0x0, 0x7},
> > > +	{"retimer_parity_err_1st", 0x0, 0x8},
> > > +	{"retimer_parity_err_2nd", 0x0, 0x9},
> > > +	{"margin_crc_parity_err", 0x0, 0xA},
> > > +	{"detect_ei_infer", 0x1, 0x5},
> > > +	{"receiver_err", 0x1, 0x6},
> > > +	{"rx_recovery_req", 0x1, 0x7},
> > > +	{"n_fts_timeout", 0x1, 0x8},
> > > +	{"framing_err", 0x1, 0x9},
> > > +	{"deskew_err", 0x1, 0xa},
> > > +	{"framing_err_in_l0", 0x1, 0xc},
> > > +	{"deskew_uncompleted_err", 0x1, 0xd},
> > > +	{"bad_tlp", 0x2, 0x0},
> > > +	{"lcrc_err", 0x2, 0x1},
> > > +	{"bad_dllp", 0x2, 0x2},
> > > +	{"replay_num_rollover", 0x2, 0x3},
> > > +	{"replay_timeout", 0x2, 0x4},
> > > +	{"rx_nak_dllp", 0x2, 0x5},
> > > +	{"tx_nak_dllp", 0x2, 0x6},
> > > +	{"retry_tlp", 0x2, 0x7},
> > > +	{"fc_timeout", 0x3, 0x0},
> > > +	{"poisoned_tlp", 0x3, 0x1},
> > > +	{"ecrc_error", 0x3, 0x2},
> > > +	{"unsupported_request", 0x3, 0x3},
> > > +	{"completer_abort", 0x3, 0x4},
> > > +	{"completion_timeout", 0x3, 0x5},
> > > +	{"ebuf_skp_add", 0x4, 0x0},
> > > +	{"ebuf_skp_del", 0x4, 0x1},
> > > +};
> > > +
> > > +/**
> > > + * struct dwc_pcie_err_inj - Store details about each error
> > > +injection supported by DWC RASDES
> > > + * @name: Name of the error that can be injected
> > > + * @err_inj_group: Group number to which the error belongs to.
> > > +Value can range from 0 - 5
> > > + * @err_inj_type: Each group can have multiple types of error  */
> > > +struct dwc_pcie_err_inj {
> > > +	const char *name;
> > > +	u32 err_inj_group;
> > > +	u32 err_inj_type;
> > > +};
> > > +
> > > +static const struct dwc_pcie_err_inj err_inj_list[] = {
> > > +	{"tx_lcrc", 0x0, 0x0},
> > > +	{"b16_crc_dllp", 0x0, 0x1},
> > > +	{"b16_crc_upd_fc", 0x0, 0x2},
> > > +	{"tx_ecrc", 0x0, 0x3},
> > > +	{"fcrc_tlp", 0x0, 0x4},
> > > +	{"parity_tsos", 0x0, 0x5},
> > > +	{"parity_skpos", 0x0, 0x6},
> > > +	{"rx_lcrc", 0x0, 0x8},
> > > +	{"rx_ecrc", 0x0, 0xb},
> > > +	{"tlp_err_seq", 0x1, 0x0},
> > > +	{"ack_nak_dllp_seq", 0x1, 0x1},
> > > +	{"ack_nak_dllp", 0x2, 0x0},
> > > +	{"upd_fc_dllp", 0x2, 0x1},
> > > +	{"nak_dllp", 0x2, 0x2},
> > > +	{"inv_sync_hdr_sym", 0x3, 0x0},
> > > +	{"com_pad_ts1", 0x3, 0x1},
> > > +	{"com_pad_ts2", 0x3, 0x2},
> > > +	{"com_fts", 0x3, 0x3},
> > > +	{"com_idl", 0x3, 0x4},
> > > +	{"end_edb", 0x3, 0x5},
> > > +	{"stp_sdp", 0x3, 0x6},
> > > +	{"com_skp", 0x3, 0x7},
> > > +	{"posted_tlp_hdr", 0x4, 0x0},
> > > +	{"non_post_tlp_hdr", 0x4, 0x1},
> > > +	{"cmpl_tlp_hdr", 0x4, 0x2},
> > > +	{"posted_tlp_data", 0x4, 0x4},
> > > +	{"non_post_tlp_data", 0x4, 0x5},
> > > +	{"cmpl_tlp_data", 0x4, 0x6},
> > > +	{"duplicate_dllp", 0x5, 0x0},
> > > +	{"nullified_tlp", 0x5, 0x1},
> > > +};
> > > +
> > > +static const u32 err_inj_type_mask[] = {
> > > +	EINJ0_TYPE,
> > > +	EINJ1_TYPE,
> > > +	EINJ2_TYPE,
> > > +	EINJ3_TYPE,
> > > +	EINJ4_TYPE,
> > > +	EINJ5_TYPE,
> > > +};
> > > +
> > > +static ssize_t lane_detect_read(struct file *file, char __user
> > > +*buf, size_t count, loff_t *ppos) {
> > > +	struct dw_pcie *pci = file->private_data;
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > > +	ssize_t off = 0;
> > > +	u32 val;
> > > +
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
> > > +	val = FIELD_GET(PIPE_DETECT_LANE, val);
> > > +	if (val)
> > > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane Detected\n");
> > > +	else
> > > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane
> > > +Undetected\n");
> > > +
> > > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf,
> > > +off); }
> > > +
> > > +static ssize_t lane_detect_write(struct file *file, const char __user *buf,
> > > +				 size_t count, loff_t *ppos)
> > > +{
> > > +	struct dw_pcie *pci = file->private_data;
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +	u32 lane, val;
> > > +
> > > +	val = kstrtou32_from_user(buf, count, 0, &lane);
> > > +	if (val)
> > > +		return val;
> > > +
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
> > > +	val &= ~(LANE_SELECT);
> > > +	val |= FIELD_PREP(LANE_SELECT, lane);
> > > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset +
> > > +SD_STATUS_L1LANE_REG, val);
> > > +
> > > +	return count;
> > > +}
> > > +
> > > +static ssize_t rx_valid_read(struct file *file, char __user *buf,
> > > +size_t count, loff_t *ppos) {
> > > +	struct dw_pcie *pci = file->private_data;
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > > +	ssize_t off = 0;
> > > +	u32 val;
> > > +
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + SD_STATUS_L1LANE_REG);
> > > +	val = FIELD_GET(PIPE_RXVALID, val);
> > > +	if (val)
> > > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "RX Valid\n");
> > > +	else
> > > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "RX
> > > +Invalid\n");
> > > +
> > > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf,
> > > +off); }
> > > +
> > > +static ssize_t rx_valid_write(struct file *file, const char __user
> > > +*buf, size_t count, loff_t *ppos) {
> > > +	return lane_detect_write(file, buf, count, ppos); }
> > > +
> > > +static void set_event_number(struct dwc_pcie_rasdes_priv *pdata, struct dw_pcie *pci,
> > > +			     struct dwc_pcie_rasdes_info *rinfo) {
> > > +	u32 val;
> > > +
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > > +	val &= ~EVENT_COUNTER_ENABLE;
> > > +	val &= ~(EVENT_COUNTER_GROUP_SELECT | EVENT_COUNTER_EVENT_SELECT);
> > > +	val |= FIELD_PREP(EVENT_COUNTER_GROUP_SELECT, event_list[pdata->idx].group_no);
> > > +	val |= FIELD_PREP(EVENT_COUNTER_EVENT_SELECT, event_list[pdata->idx].event_no);
> > > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset +
> > > +RAS_DES_EVENT_COUNTER_CTRL_REG, val); }
> > > +
> > > +static ssize_t counter_enable_read(struct file *file, char __user
> > > +*buf, size_t count, loff_t *ppos) {
> > > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > > +	struct dw_pcie *pci = pdata->pci;
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > > +	ssize_t off = 0;
> > > +	u32 val;
> > > +
> > > +	mutex_lock(&rinfo->reg_lock);
> > > +	set_event_number(pdata, pci, rinfo);
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > > +	mutex_unlock(&rinfo->reg_lock);
> > > +	val = FIELD_GET(EVENT_COUNTER_STATUS, val);
> > > +	if (val)
> > > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter Enabled\n");
> > > +	else
> > > +		off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter
> > > +Disabled\n");
> > > +
> > > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf,
> > > +off); }
> > > +
> > > +static ssize_t counter_enable_write(struct file *file, const char __user *buf,
> > > +				    size_t count, loff_t *ppos) {
> > > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > > +	struct dw_pcie *pci = pdata->pci;
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +	u32 val, enable;
> > > +
> > > +	val = kstrtou32_from_user(buf, count, 0, &enable);
> > > +	if (val)
> > > +		return val;
> > > +
> > > +	mutex_lock(&rinfo->reg_lock);
> > > +	set_event_number(pdata, pci, rinfo);
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > > +	if (enable)
> > > +		val |= FIELD_PREP(EVENT_COUNTER_ENABLE, PER_EVENT_ON);
> > > +	else
> > > +		val |= FIELD_PREP(EVENT_COUNTER_ENABLE, PER_EVENT_OFF);
> > > +
> > > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
> > > +	mutex_unlock(&rinfo->reg_lock);
> > > +
> > > +	return count;
> > > +}
> > > +
> > > +static ssize_t counter_lane_read(struct file *file, char __user
> > > +*buf, size_t count, loff_t *ppos) {
> > > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > > +	struct dw_pcie *pci = pdata->pci;
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > > +	ssize_t off = 0;
> > > +	u32 val;
> > > +
> > > +	mutex_lock(&rinfo->reg_lock);
> > > +	set_event_number(pdata, pci, rinfo);
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > > +	mutex_unlock(&rinfo->reg_lock);
> > > +	val = FIELD_GET(EVENT_COUNTER_LANE_SELECT, val);
> > > +	off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Lane:
> > > +%d\n", val);
> > > +
> > > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf,
> > > +off); }
> > > +
> > > +static ssize_t counter_lane_write(struct file *file, const char __user *buf,
> > > +				  size_t count, loff_t *ppos)
> > > +{
> > > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > > +	struct dw_pcie *pci = pdata->pci;
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +	u32 val, lane;
> > > +
> > > +	val = kstrtou32_from_user(buf, count, 0, &lane);
> > > +	if (val)
> > > +		return val;
> > > +
> > > +	mutex_lock(&rinfo->reg_lock);
> > > +	set_event_number(pdata, pci, rinfo);
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
> > > +	val &= ~(EVENT_COUNTER_LANE_SELECT);
> > > +	val |= FIELD_PREP(EVENT_COUNTER_LANE_SELECT, lane);
> > > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
> > > +	mutex_unlock(&rinfo->reg_lock);
> > > +
> > > +	return count;
> > > +}
> > > +
> > > +static ssize_t counter_value_read(struct file *file, char __user
> > > +*buf, size_t count, loff_t *ppos) {
> > > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > > +	struct dw_pcie *pci = pdata->pci;
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +	char debugfs_buf[DWC_DEBUGFS_BUF_MAX];
> > > +	ssize_t off = 0;
> > > +	u32 val;
> > > +
> > > +	mutex_lock(&rinfo->reg_lock);
> > > +	set_event_number(pdata, pci, rinfo);
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_DATA_REG);
> > > +	mutex_unlock(&rinfo->reg_lock);
> > > +	off += scnprintf(debugfs_buf, DWC_DEBUGFS_BUF_MAX - off, "Counter
> > > +value: %d\n", val);
> > > +
> > > +	return simple_read_from_buffer(buf, count, ppos, debugfs_buf,
> > > +off); }
> > > +
> > > +static ssize_t err_inj_write(struct file *file, const char __user
> > > +*buf, size_t count, loff_t *ppos) {
> > > +	struct dwc_pcie_rasdes_priv *pdata = file->private_data;
> > > +	struct dw_pcie *pci = pdata->pci;
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +	u32 val, counter, vc_num, err_group, type_mask;
> > > +	int val_diff = 0;
> > > +	char *kern_buf;
> > > +
> > > +	err_group = err_inj_list[pdata->idx].err_inj_group;
> > > +	type_mask = err_inj_type_mask[err_group];
> > > +
> > > +	kern_buf = memdup_user_nul(buf, count);
> > > +	if (IS_ERR(kern_buf))
> > > +		return PTR_ERR(kern_buf);
> > > +
> > > +	if (err_group == 4) {
> > > +		val = sscanf(kern_buf, "%u %d %u", &counter, &val_diff, &vc_num);
> > > +		if ((val != 3) || (val_diff < -4095 || val_diff > 4095)) {
> > > +			kfree(kern_buf);
> > > +			return -EINVAL;
> > > +		}
> > > +	} else if (err_group == 1) {
> > > +		val = sscanf(kern_buf, "%u %d", &counter, &val_diff);
> > > +		if ((val != 2) || (val_diff < -4095 || val_diff > 4095)) {
> > > +			kfree(kern_buf);
> > > +			return -EINVAL;
> > > +		}
> > > +	} else {
> > > +		val = kstrtou32(kern_buf, 0, &counter);
> > > +		if (val) {
> > > +			kfree(kern_buf);
> > > +			return val;
> > > +		}
> > > +	}
> > > +
> > > +	val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + ERR_INJ0_OFF + (0x4 * err_group));
> > > +	val &= ~(type_mask | EINJ_COUNT);
> > > +	val |= ((err_inj_list[pdata->idx].err_inj_type << EINJ_TYPE_SHIFT) & type_mask);
> > > +	val |= FIELD_PREP(EINJ_COUNT, counter);
> > > +
> > > +	if (err_group == 1 || err_group == 4) {
> > > +		val &= ~(EINJ_VAL_DIFF);
> > > +		val |= FIELD_PREP(EINJ_VAL_DIFF, val_diff);
> > > +	}
> > > +	if (err_group == 4) {
> > > +		val &= ~(EINJ_VC_NUM);
> > > +		val |= FIELD_PREP(EINJ_VC_NUM, vc_num);
> > > +	}
> > > +
> > > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + ERR_INJ0_OFF + (0x4 * err_group), val);
> > > +	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset +
> > > +ERR_INJ_ENABLE_REG,
> > > +(0x1 << err_group));
> > > +
> > > +	kfree(kern_buf);
> > > +	return count;
> > > +}
> > > +
> > > +#define dwc_debugfs_create(name)			\
> > > +debugfs_create_file(#name, 0644, rasdes_debug, pci,	\
> > > +			&dbg_ ## name ## _fops)
> > > +
> > > +#define DWC_DEBUGFS_FOPS(name)					\
> > > +static const struct file_operations dbg_ ## name ## _fops = {	\
> > > +	.open = simple_open,				\
> > > +	.read = name ## _read,				\
> > > +	.write = name ## _write				\
> > > +}
> > > +
> > > +DWC_DEBUGFS_FOPS(lane_detect);
> > > +DWC_DEBUGFS_FOPS(rx_valid);
> > > +
> > > +static const struct file_operations dwc_pcie_counter_enable_ops = {
> > > +	.open = simple_open,
> > > +	.read = counter_enable_read,
> > > +	.write = counter_enable_write,
> > > +};
> > > +
> > > +static const struct file_operations dwc_pcie_counter_lane_ops = {
> > > +	.open = simple_open,
> > > +	.read = counter_lane_read,
> > > +	.write = counter_lane_write,
> > > +};
> > > +
> > > +static const struct file_operations dwc_pcie_counter_value_ops = {
> > > +	.open = simple_open,
> > > +	.read = counter_value_read,
> > > +};
> > > +
> > > +static const struct file_operations dwc_pcie_err_inj_ops = {
> > > +	.open = simple_open,
> > > +	.write = err_inj_write,
> > > +};
> > > +
> > > +void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci) {
> > > +	struct dwc_pcie_rasdes_info *rinfo = pci->rasdes_info;
> > > +
> > > +	debugfs_remove_recursive(rinfo->rasdes_dir);
> > > +	mutex_destroy(&rinfo->reg_lock);
> > > +}
> > > +
> > > +int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci) {
> > > +	struct dentry *dir, *rasdes_debug, *rasdes_err_inj, *rasdes_event_counter, *rasdes_events;
> > > +	struct dwc_pcie_rasdes_info *rasdes_info;
> > > +	struct dwc_pcie_rasdes_priv *priv_tmp;
> > > +	char dirname[DWC_DEBUGFS_BUF_MAX];
> > > +	struct device *dev = pci->dev;
> > > +	int ras_cap, i, ret;
> > > +
> > > +	ras_cap = dw_pcie_find_vsec_capability(pci, DW_PCIE_VSEC_EXT_CAP_RAS_DES);
> > > +	if (!ras_cap) {
> > > +		dev_dbg(dev, "No RASDES capability available\n");
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	rasdes_info = devm_kzalloc(dev, sizeof(*rasdes_info), GFP_KERNEL);
> > > +	if (!rasdes_info)
> > > +		return -ENOMEM;
> > > +
> > > +	/* Create main directory for each platform driver */
> > > +	snprintf(dirname, DWC_DEBUGFS_BUF_MAX, "dwc_pcie_%s", dev_name(dev));
> > > +	dir = debugfs_create_dir(dirname, NULL);
> > > +	if (IS_ERR(dir))
> > > +		return PTR_ERR(dir);
> > > +
> > > +	/* Create subdirectories for Debug, Error injection, Statistics */
> > > +	rasdes_debug = debugfs_create_dir("rasdes_debug", dir);
> > > +	rasdes_err_inj = debugfs_create_dir("rasdes_err_inj", dir);
> > > +	rasdes_event_counter = debugfs_create_dir("rasdes_event_counter",
> > > +dir);
> > > +
> > > +	mutex_init(&rasdes_info->reg_lock);
> > > +	rasdes_info->ras_cap_offset = ras_cap;
> > > +	rasdes_info->rasdes_dir = dir;
> > > +	pci->rasdes_info = rasdes_info;
> > > +
> > > +	/* Create debugfs files for Debug subdirectory */
> > > +	dwc_debugfs_create(lane_detect);
> > > +	dwc_debugfs_create(rx_valid);
> > > +
> > > +	/* Create debugfs files for Error injection subdirectory */
> > > +	for (i = 0; i < ARRAY_SIZE(err_inj_list); i++) {
> > > +		priv_tmp = devm_kzalloc(dev, sizeof(*priv_tmp), GFP_KERNEL);
> > > +		if (!priv_tmp) {
> > > +			ret = -ENOMEM;
> > > +			goto err_deinit;
> > > +		}
> > > +
> > > +		priv_tmp->idx = i;
> > > +		priv_tmp->pci = pci;
> > > +		debugfs_create_file(err_inj_list[i].name, 0200, rasdes_err_inj, priv_tmp,
> > > +				    &dwc_pcie_err_inj_ops);
> > > +	}
> > > +
> > > +	/* Create debugfs files for Statistical counter subdirectory */
> > > +	for (i = 0; i < ARRAY_SIZE(event_list); i++) {
> > > +		priv_tmp = devm_kzalloc(dev, sizeof(*priv_tmp), GFP_KERNEL);
> > > +		if (!priv_tmp) {
> > > +			ret = -ENOMEM;
> > > +			goto err_deinit;
> > > +		}
> > > +
> > > +		priv_tmp->idx = i;
> > > +		priv_tmp->pci = pci;
> > > +		rasdes_events = debugfs_create_dir(event_list[i].name, rasdes_event_counter);
> > > +		if (event_list[i].group_no == 0 || event_list[i].group_no == 4) {
> > > +			debugfs_create_file("lane_select", 0644, rasdes_events,
> > > +					    priv_tmp, &dwc_pcie_counter_lane_ops);
> > > +		}
> > > +		debugfs_create_file("counter_value", 0444, rasdes_events, priv_tmp,
> > > +				    &dwc_pcie_counter_value_ops);
> > > +		debugfs_create_file("counter_enable", 0644, rasdes_events, priv_tmp,
> > > +				    &dwc_pcie_counter_enable_ops);
> > > +	}
> > > +
> > > +	return 0;
> > > +
> > > +err_deinit:
> > > +	dwc_pcie_rasdes_debugfs_deinit(pci);
> > > +	return ret;
> > > +}
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.h
> > > b/drivers/pci/controller/dwc/pcie-designware-debugfs.h
> > > new file mode 100644
> > > index 000000000000..e69de29bb2d1
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > > b/drivers/pci/controller/dwc/pcie-designware.h
> > > index 98a057820bc7..ed0f26d69626 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -260,6 +260,8 @@
> > >
> > >  #define PCIE_RAS_DES_EVENT_COUNTER_DATA		0xc
> > >
> > > +#define DW_PCIE_VSEC_EXT_CAP_RAS_DES		0x2
> > > +
> > >  /*
> > >   * The default address offset between dbi_base and atu_base. Root controller
> > >   * drivers are not required to initialize atu_base if the offset
> > > matches this @@ -463,6 +465,7 @@ struct dw_pcie {
> > >  	struct reset_control_bulk_data	core_rsts[DW_PCIE_NUM_CORE_RSTS];
> > >  	struct gpio_desc		*pe_rst;
> > >  	bool			suspended;
> > > +	void			*rasdes_info;
> > >  };
> > >
> > >  #define to_dw_pcie_from_pp(port) container_of((port), struct
> > > dw_pcie,
> > > pp) @@ -796,4 +799,18 @@ dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
> > >  	return NULL;
> > >  }
> > >  #endif
> > > +
> > > +#ifdef CONFIG_PCIE_DW_DEBUGFS
> > > +int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci); void
> > > +dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci); #else static
> > > +inline int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci) {
> > > +	return 0;
> > > +}
> > > +static inline void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie
> > > +*pci) { } #endif
> > > +
> > >  #endif /* _PCIE_DESIGNWARE_H */
> > > --
> > > 2.17.1
> > >
> >
> > --
> > Fan Ni (From gmail)

Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Manivannan Sadhasivam]

On Wed, Dec 11, 2024 at 08:43:30AM -0600, Bjorn Helgaas wrote:
> On Wed, Dec 11, 2024 at 05:15:50PM +0530, Shradha Todi wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > Sent: 06 December 2024 21:43
> > > To: Shradha Todi <shradha.t@samsung.com>
> > > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> > > kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> > > fan.ni@samsung.com; a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> > > quic_krichai@quicinc.com; gost.dev@samsung.com
> > > Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> > > 
> > > On Fri, Dec 06, 2024 at 01:14:55PM +0530, Shradha Todi wrote:
> > > > Add vendor specific extended configuration space capability search API
> > > > using struct dw_pcie pointer for DW controllers.
> > > >
> > > > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-designware.c | 16 ++++++++++++++++
> > > > drivers/pci/controller/dwc/pcie-designware.h |  1 +
> > > >  2 files changed, 17 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > index 6d6cbc8b5b2c..41230c5e4a53 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > @@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> > > >  	return 0;
> > > >  }
> > > >
> > > > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
> > > 
> > > To make sure that we find a VSEC ID that corresponds to the
> > > expected vendor, I think this interface needs to be the same
> > > as pci_find_vsec_capability().  In particular, it needs to take a
> > > "u16 vendor"
> >
> > As per my understanding, Synopsys is the vendor here when we talk
> > about vsec capabilities.  VSEC cap IDs are fixed for each vendor
> > (eg: For Synopsys Designware controllers, 0x2 is always RAS CAP, 0x4
> > is always PTM responder and so on).
> 
> For VSEC, the vendor that matters is the one identified at 0x0 in
> config space.  That's why pci_find_vsec_capability() checks the
> supplied "vendor" against "dev->vendor".
> 
> > So no matter if the DWC IP is being integrated by Samsung, NVDIA or
> > Qcom, the vendor specific CAP IDs will remain constant. Now since
> > this function is being written as part of designware file, the
> > control will reach here only when the PCIe IP is DWC. So, we don't
> > really require a vendor ID to be checked here. EG: If 0x2 VSEC ID is
> > present in any DWC controller, it means RAS is supported. Please
> > correct me if I'm wrong.
> 
> In this case, the Vendor ID is typically Samsung, NVIDIA, Qcom, etc.,
> even though it may contain Synopsys DWC IP.  Each vendor assigns VSEC
> IDs independently, so VSEC ID 0x2 may mean something different to
> Samsung than it does to NVIDIA or Qcom.
> 
> PCIe r6.0, sec 7.9.5 has the details, but the important part is this:
> 
>   With a PCI Express Function, the structure and definition of the
>   vendor-specific Registers area is determined by the vendor indicated
>   by the Vendor ID field located at byte offset 00h in PCI-compatible
>   Configuration Space.
> 
> There IS a separate DVSEC ("Designated Vendor-Specific") Capability;
> see sec 7.9.6.  That one does include a DVSEC Vendor ID in the
> Capability itself, and this would make more sense for this situation.
> 
> If Synopsys assigned DVSEC ID 0x2 from the Synopsys namespace for RAS,
> then devices from Samsung, NVIDIA, Qcom, etc., could advertise a DVSEC
> Capability that contained a DVSEC Vendor ID of PCI_VENDOR_ID_SYNOPSYS
> with DVSEC ID 0x2, and all those devices could easily locate it.
> 
> Unfortunately Samsung et al used VSEC instead of DVSEC, so we're stuck
> with having to specify the device vendor and the VSEC ID assigned by
> that vendor, and those VSEC IDs might be different per vendor.
> 

Atleast on Qcom platforms, VSEC_ID is 0x2 for RAS. But this is not guaranteed to
be the same as per the PCIe spec as you mentioned. Though, I think it is safe to
go with it since we have seen the same IDs on 2 platforms (my gut feeling is
that it is going to be the same on other DWC vendor platforms as well). If we
encounter different IDs, then we can add vendor id check.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Bjorn Helgaas]

On Wed, Jan 15, 2025 at 08:57:42PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Dec 11, 2024 at 08:43:30AM -0600, Bjorn Helgaas wrote:
> > On Wed, Dec 11, 2024 at 05:15:50PM +0530, Shradha Todi wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > > Sent: 06 December 2024 21:43
> > > > To: Shradha Todi <shradha.t@samsung.com>
> > > > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> > > > kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> > > > fan.ni@samsung.com; a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> > > > quic_krichai@quicinc.com; gost.dev@samsung.com
> > > > Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> > > > 
> > > > On Fri, Dec 06, 2024 at 01:14:55PM +0530, Shradha Todi wrote:
> > > > > Add vendor specific extended configuration space capability search API
> > > > > using struct dw_pcie pointer for DW controllers.
> > > > >
> > > > > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > > > > ---
> > > > >  drivers/pci/controller/dwc/pcie-designware.c | 16 ++++++++++++++++
> > > > > drivers/pci/controller/dwc/pcie-designware.h |  1 +
> > > > >  2 files changed, 17 insertions(+)
> > > > >
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > index 6d6cbc8b5b2c..41230c5e4a53 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > @@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> > > > >  	return 0;
> > > > >  }
> > > > >
> > > > > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
> > > > 
> > > > To make sure that we find a VSEC ID that corresponds to the
> > > > expected vendor, I think this interface needs to be the same
> > > > as pci_find_vsec_capability().  In particular, it needs to take a
> > > > "u16 vendor"
> > >
> > > As per my understanding, Synopsys is the vendor here when we talk
> > > about vsec capabilities.  VSEC cap IDs are fixed for each vendor
> > > (eg: For Synopsys Designware controllers, 0x2 is always RAS CAP, 0x4
> > > is always PTM responder and so on).
> > 
> > For VSEC, the vendor that matters is the one identified at 0x0 in
> > config space.  That's why pci_find_vsec_capability() checks the
> > supplied "vendor" against "dev->vendor".
> > 
> > > So no matter if the DWC IP is being integrated by Samsung, NVDIA or
> > > Qcom, the vendor specific CAP IDs will remain constant. Now since
> > > this function is being written as part of designware file, the
> > > control will reach here only when the PCIe IP is DWC. So, we don't
> > > really require a vendor ID to be checked here. EG: If 0x2 VSEC ID is
> > > present in any DWC controller, it means RAS is supported. Please
> > > correct me if I'm wrong.
> > 
> > In this case, the Vendor ID is typically Samsung, NVIDIA, Qcom, etc.,
> > even though it may contain Synopsys DWC IP.  Each vendor assigns VSEC
> > IDs independently, so VSEC ID 0x2 may mean something different to
> > Samsung than it does to NVIDIA or Qcom.
> > 
> > PCIe r6.0, sec 7.9.5 has the details, but the important part is this:
> > 
> >   With a PCI Express Function, the structure and definition of the
> >   vendor-specific Registers area is determined by the vendor indicated
> >   by the Vendor ID field located at byte offset 00h in PCI-compatible
> >   Configuration Space.
> > 
> > There IS a separate DVSEC ("Designated Vendor-Specific") Capability;
> > see sec 7.9.6.  That one does include a DVSEC Vendor ID in the
> > Capability itself, and this would make more sense for this situation.
> > 
> > If Synopsys assigned DVSEC ID 0x2 from the Synopsys namespace for RAS,
> > then devices from Samsung, NVIDIA, Qcom, etc., could advertise a DVSEC
> > Capability that contained a DVSEC Vendor ID of PCI_VENDOR_ID_SYNOPSYS
> > with DVSEC ID 0x2, and all those devices could easily locate it.
> > 
> > Unfortunately Samsung et al used VSEC instead of DVSEC, so we're stuck
> > with having to specify the device vendor and the VSEC ID assigned by
> > that vendor, and those VSEC IDs might be different per vendor.
> 
> Atleast on Qcom platforms, VSEC_ID is 0x2 for RAS. But this is not
> guaranteed to be the same as per the PCIe spec as you mentioned.
> Though, I think it is safe to go with it since we have seen the same
> IDs on 2 platforms (my gut feeling is that it is going to be the
> same on other DWC vendor platforms as well). If we encounter
> different IDs, then we can add vendor id check.

This series uses:

  dw_pcie_find_vsec_capability(pci, DW_PCIE_VSEC_EXT_CAP_RAS_DES)

in dwc_pcie_rasdes_debugfs_init(), but I don't see any calls of that
function yet.  If it is called only from code that already knows the
device vendor has assigned VSEC ID 0x02 for the DWC RAS functionality,
I guess it is "safe".

But I think it would be a bad idea because it perpetuates the
misunderstanding that DesignWare can independently claim ownership of
VSEC ID 0x02 for *all* vendors, and other vendors have already used
VSEC ID 0x02 for different things (examples at [1]).  If any of them
incorporates this DWC IP, they will have to use a different VSEC ID to
avoid a collision with their existing VSEC ID 0x02.

Bjorn

[1] https://lore.kernel.org/r/20241209222938.3219364-1-helgaas@kernel.org/

Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Manivannan Sadhasivam]

On Wed, Jan 15, 2025 at 10:12:01AM -0600, Bjorn Helgaas wrote:
> On Wed, Jan 15, 2025 at 08:57:42PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Dec 11, 2024 at 08:43:30AM -0600, Bjorn Helgaas wrote:
> > > On Wed, Dec 11, 2024 at 05:15:50PM +0530, Shradha Todi wrote:
> > > > > -----Original Message-----
> > > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > > > Sent: 06 December 2024 21:43
> > > > > To: Shradha Todi <shradha.t@samsung.com>
> > > > > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> > > > > kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> > > > > fan.ni@samsung.com; a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> > > > > quic_krichai@quicinc.com; gost.dev@samsung.com
> > > > > Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> > > > > 
> > > > > On Fri, Dec 06, 2024 at 01:14:55PM +0530, Shradha Todi wrote:
> > > > > > Add vendor specific extended configuration space capability search API
> > > > > > using struct dw_pcie pointer for DW controllers.
> > > > > >
> > > > > > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > > > > > ---
> > > > > >  drivers/pci/controller/dwc/pcie-designware.c | 16 ++++++++++++++++
> > > > > > drivers/pci/controller/dwc/pcie-designware.h |  1 +
> > > > > >  2 files changed, 17 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > index 6d6cbc8b5b2c..41230c5e4a53 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > @@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> > > > > >  	return 0;
> > > > > >  }
> > > > > >
> > > > > > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
> > > > > 
> > > > > To make sure that we find a VSEC ID that corresponds to the
> > > > > expected vendor, I think this interface needs to be the same
> > > > > as pci_find_vsec_capability().  In particular, it needs to take a
> > > > > "u16 vendor"
> > > >
> > > > As per my understanding, Synopsys is the vendor here when we talk
> > > > about vsec capabilities.  VSEC cap IDs are fixed for each vendor
> > > > (eg: For Synopsys Designware controllers, 0x2 is always RAS CAP, 0x4
> > > > is always PTM responder and so on).
> > > 
> > > For VSEC, the vendor that matters is the one identified at 0x0 in
> > > config space.  That's why pci_find_vsec_capability() checks the
> > > supplied "vendor" against "dev->vendor".
> > > 
> > > > So no matter if the DWC IP is being integrated by Samsung, NVDIA or
> > > > Qcom, the vendor specific CAP IDs will remain constant. Now since
> > > > this function is being written as part of designware file, the
> > > > control will reach here only when the PCIe IP is DWC. So, we don't
> > > > really require a vendor ID to be checked here. EG: If 0x2 VSEC ID is
> > > > present in any DWC controller, it means RAS is supported. Please
> > > > correct me if I'm wrong.
> > > 
> > > In this case, the Vendor ID is typically Samsung, NVIDIA, Qcom, etc.,
> > > even though it may contain Synopsys DWC IP.  Each vendor assigns VSEC
> > > IDs independently, so VSEC ID 0x2 may mean something different to
> > > Samsung than it does to NVIDIA or Qcom.
> > > 
> > > PCIe r6.0, sec 7.9.5 has the details, but the important part is this:
> > > 
> > >   With a PCI Express Function, the structure and definition of the
> > >   vendor-specific Registers area is determined by the vendor indicated
> > >   by the Vendor ID field located at byte offset 00h in PCI-compatible
> > >   Configuration Space.
> > > 
> > > There IS a separate DVSEC ("Designated Vendor-Specific") Capability;
> > > see sec 7.9.6.  That one does include a DVSEC Vendor ID in the
> > > Capability itself, and this would make more sense for this situation.
> > > 
> > > If Synopsys assigned DVSEC ID 0x2 from the Synopsys namespace for RAS,
> > > then devices from Samsung, NVIDIA, Qcom, etc., could advertise a DVSEC
> > > Capability that contained a DVSEC Vendor ID of PCI_VENDOR_ID_SYNOPSYS
> > > with DVSEC ID 0x2, and all those devices could easily locate it.
> > > 
> > > Unfortunately Samsung et al used VSEC instead of DVSEC, so we're stuck
> > > with having to specify the device vendor and the VSEC ID assigned by
> > > that vendor, and those VSEC IDs might be different per vendor.
> > 
> > Atleast on Qcom platforms, VSEC_ID is 0x2 for RAS. But this is not
> > guaranteed to be the same as per the PCIe spec as you mentioned.
> > Though, I think it is safe to go with it since we have seen the same
> > IDs on 2 platforms (my gut feeling is that it is going to be the
> > same on other DWC vendor platforms as well). If we encounter
> > different IDs, then we can add vendor id check.
> 
> This series uses:
> 
>   dw_pcie_find_vsec_capability(pci, DW_PCIE_VSEC_EXT_CAP_RAS_DES)
> 
> in dwc_pcie_rasdes_debugfs_init(), but I don't see any calls of that
> function yet.

I guess that the caller got missed unintentionally in patch 2/2.

>  If it is called only from code that already knows the
> device vendor has assigned VSEC ID 0x02 for the DWC RAS functionality,
> I guess it is "safe".
> 

It should be called from the DWC code driver (pcie-desginware-host.c).

> But I think it would be a bad idea because it perpetuates the
> misunderstanding that DesignWare can independently claim ownership of
> VSEC ID 0x02 for *all* vendors, and other vendors have already used
> VSEC ID 0x02 for different things (examples at [1]).  If any of them
> incorporates this DWC IP, they will have to use a different VSEC ID to
> avoid a collision with their existing VSEC ID 0x02.
> 

Fair enough. I was trying to avoid updating the vendor id table for enabling the
RAS DES debug feature, but I think it would be worth doing so (perf driver is
also doing the same).

So yeah, I'm OK with the idea of having the vendor_id check in this API.

(Also, I don't see the VSEC_IDs defined in the DWC reference manual that I got
access to).

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Bjorn Helgaas]

On Wed, Jan 15, 2025 at 09:59:53PM +0530, Manivannan Sadhasivam wrote:
> ...

> (Also, I don't see the VSEC_IDs defined in the DWC reference manual
> that I got access to).

Haha, yeah, that's because DWC can only define VSEC IDs for devices
with PCI_VENDOR_ID_SYNOPSYS, and if they want to sell IP for use in
devices with other Vendor IDs, it's up to those vendors to define the
VSEC ID in their products.  That's exactly the issue here :)

Bjorn

Re: [PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support in DWC

[Manivannan Sadhasivam]

On Fri, Dec 06, 2024 at 01:14:56PM +0530, Shradha Todi wrote:
> Add support to use the RASDES feature of DesignWare PCIe controller
> using debugfs entries.
> 
> RASDES is a vendor specific extended PCIe capability which reads the
> current hardware internal state of PCIe device. Following primary
> features are provided to userspace via debugfs:
> - Debug registers
> - Error injection
> - Statistical counters
> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  Documentation/ABI/testing/debugfs-dwc-pcie    | 143 +++++
>  drivers/pci/controller/dwc/Kconfig            |  11 +
>  drivers/pci/controller/dwc/Makefile           |   1 +
>  .../controller/dwc/pcie-designware-debugfs.c  | 544 ++++++++++++++++++
>  .../controller/dwc/pcie-designware-debugfs.h  |   0
>  drivers/pci/controller/dwc/pcie-designware.h  |  17 +
>  6 files changed, 716 insertions(+)
>  create mode 100644 Documentation/ABI/testing/debugfs-dwc-pcie
>  create mode 100644 drivers/pci/controller/dwc/pcie-designware-debugfs.c
>  create mode 100644 drivers/pci/controller/dwc/pcie-designware-debugfs.h
> 
> diff --git a/Documentation/ABI/testing/debugfs-dwc-pcie b/Documentation/ABI/testing/debugfs-dwc-pcie
> new file mode 100644
> index 000000000000..7da73ac8d40c
> --- /dev/null
> +++ b/Documentation/ABI/testing/debugfs-dwc-pcie
> @@ -0,0 +1,143 @@
> +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/lane_detect
> +Date:		December 2024
> +Contact:	Shradha Todi <shradha.t@samsung.com>
> +Description:	(RW) Write the lane number to be checked for detection.	Read
> +		will dump whether PHY indicates receiver detection on the

s/dump/return - for other similar attributes as well.

> +		selected lane.

So what is the behavior if the attribute is read before writing (selecting) any
lane? Same comment for other similar attributes below.

> +
> +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_debug/rx_valid
> +Date:		December 2024
> +Contact:	Shradha Todi <shradha.t@samsung.com>
> +Description:	(RW) Write the lane number to be checked as valid or invalid. Read
> +		will dump the status of PIPE RXVALID signal of the selected lane.
> +
> +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_event_counters/<event>/counter_enable
> +Date:		December 2024
> +Contact:	Shradha Todi <shradha.t@samsung.com>
> +Description:	rasdes_event_counters is the directory which can be used to collect
> +		statistical data about the number of times a certain event has occurred
> +		in the controller. The list of possible events are:
> +
> +		1) EBUF Overflow
> +		2) EBUF Underrun
> +		3) Decode Error
> +		4) Running Disparity Error
> +		5) SKP OS Parity Error
> +		6) SYNC Header Error
> +		7) Rx Valid De-assertion
> +		8) CTL SKP OS Parity Error
> +		9) 1st Retimer Parity Error
> +		10) 2nd Retimer Parity Error
> +		11) Margin CRC and Parity Error
> +		12) Detect EI Infer
> +		13) Receiver Error
> +		14) RX Recovery Req
> +		15) N_FTS Timeout
> +		16) Framing Error
> +		17) Deskew Error
> +		18) Framing Error In L0
> +		19) Deskew Uncompleted Error
> +		20) Bad TLP
> +		21) LCRC Error
> +		22) Bad DLLP
> +		23) Replay Number Rollover
> +		24) Replay Timeout
> +		25) Rx Nak DLLP
> +		26) Tx Nak DLLP
> +		27) Retry TLP
> +		28) FC Timeout
> +		29) Poisoned TLP
> +		30) ECRC Error
> +		31) Unsupported Request
> +		32) Completer Abort
> +		33) Completion Timeout
> +		34) EBUF SKP Add
> +		35) EBUF SKP Del
> +
> +		counter_enable is RW. Write 1 to enable the event counter and write 0 to
> +		disable the event counter. Read will dump whether the counter is currently
> +		enabled	or disabled.
> +
> +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_event_counters/<event>/counter_value
> +Date:		December 2024
> +Contact:	Shradha Todi <shradha.t@samsung.com>
> +Description:	(RO) Read will dump the current value of the event counter. To reset the counter,
> +		we can disable and re-enable the counter.

Avoid using 'we' in ABI documentation. This could be reworded as,

"To reset the counter, counter should be disabled and enabled back using
the 'counter_enable' attribute." 

> +
> +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_event_counters/<event>/lane_select
> +Date:		December 2024
> +Contact:	Shradha Todi <shradha.t@samsung.com>
> +Description:	(RW) Some lanes in the event list are lane specific events. These include
> +		events 1) - 11) and 34) - 35).
> +		Write lane number for which counter needs to be enabled/disabled/dumped.
> +		Read will dump the current selected lane number.
> +
> +What:		/sys/kernel/debug/dwc_pcie_<dev>/rasdes_err_inj/<error>
> +Date:		December 2024
> +Contact:	Shradha Todi <shradha.t@samsung.com>
> +Description:	rasdes_err_inj is the directory which can be used to inject errors in the
> +		system. The possible errors that can be injected are:
> +
> +		1) TLP LCRC error injection TX Path - tx_lcrc
> +		2) 16b CRC error injection of ACK/NAK DLLP - b16_crc_dllp
> +		3) 16b CRC error injection of Update-FC DLLP - b16_crc_upd_fc
> +		4) TLP ECRC error injection TX Path - tx_ecrc
> +		5) TLP's FCRC error injection TX Path - fcrc_tlp
> +		6) Parity error of TSOS - parity_tsos
> +		7) Parity error on SKPOS - parity_skpos
> +		8) LCRC error injection RX Path - rx_lcrc
> +		9) ECRC error injection RX Path - rx_ecrc
> +		10) TLPs SEQ# error - tlp_err_seq
> +		11) DLLPS ACK/NAK SEQ# error - ack_nak_dllp_seq
> +		12) ACK/NAK DLLPs transmission block - ack_nak_dllp
> +		13) UpdateFC DLLPs transmission block - upd_fc_dllp
> +		14) Always transmission for NAK DLLP - nak_dllp
> +		15) Invert SYNC header - inv_sync_hdr_sym
> +		16) COM/PAD TS1 order set - com_pad_ts1
> +		17) COM/PAD TS2 order set - com_pad_ts2
> +		18) COM/FTS FTS order set - com_fts
> +		19) COM/IDL E-idle order set - com_idl
> +		20) END/EDB symbol - end_edb
> +		21) STP/SDP symbol - stp_sdp
> +		22) COM/SKP SKP order set - com_skp
> +		23) Posted TLP Header credit value control - posted_tlp_hdr
> +		24) Non-Posted TLP Header credit value control - non_post_tlp_hdr
> +		25) Completion TLP Header credit value control - cmpl_tlp_hdr
> +		26) Posted TLP Data credit value control - posted_tlp_data
> +		27) Non-Posted TLP Data credit value control - non_post_tlp_data
> +		28) Completion TLP Data credit value control - cmpl_tlp_data
> +		29) Generates duplicate TLPs - duplicate_dllp
> +		30) Generates Nullified TLPs - nullified_tlp
> +
> +		Each of the possible errors are WO files. Write to the file will prepare

s/file/attributes

> +		controller to inject the respective error in the next transmission of data.
> +		Parameter required to write will change in the following ways:
> +
> +		i) Errors 9) - 10) are sequence errors. The write command for these will be
> +
> +			echo <count> <diff> > /sys/kernel/debug/dwc_pcie_<dev>/rasdes_err_inj/<error>
> +
> +			<count>
> +				Number of errors to be injected
> +			<diff>
> +				The difference to add or subtract from natural sequence number to
> +				generate sequence error. Range (-4095 : 4095)
> +
> +		ii) Errors 23) - 28) are credit value error insertions. Write command:
> +
> +			echo <count> <diff> <vc> > /sys/kernel/debug/dwc_pcie_<dev>/rasdes_err_inj/<error>
> +
> +			<count>
> +				Number of errors to be injected
> +			<diff>
> +				The difference to add or subtract from UpdateFC credit value.
> +				Range (-4095 : 4095)
> +			<vc>
> +				Target VC number
> +
> +		iii) All other errors. Write command:
> +
> +			echo <count> > /sys/kernel/debug/dwc_pcie_<dev>/rasdes_err_inj/<error>
> +
> +			<count>
> +				Number of errors to be injected
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index b6d6778b0698..9ab8d724fe0d 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -6,6 +6,17 @@ menu "DesignWare-based PCIe controllers"
>  config PCIE_DW
>  	bool
>  
> +config PCIE_DW_DEBUGFS
> +	default y
> +	depends on DEBUG_FS
> +	depends on PCIE_DW_HOST || PCIE_DW_EP
> +	bool "DWC PCIe debugfs entries"
> +	help
> +	  Enables debugfs entries for the DWC PCIe Controller.
> +	  These entries make use of the RAS features in the DW

s/DW/DWC

> +	  controller to help in debug, error injection and statistical
> +	  counters

Use 80 column width.

> +
>  config PCIE_DW_HOST
>  	bool
>  	select PCIE_DW
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index a8308d9ea986..54565eedc52c 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> +obj-$(CONFIG_PCIE_DW_DEBUGFS) += pcie-designware-debugfs.o
>  obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>  obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.c b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> new file mode 100644
> index 000000000000..a93e29993f75
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> @@ -0,0 +1,544 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Synopsys DesignWare PCIe controller debugfs driver
> + *
> + * Copyright (C) 2024 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com
> + *
> + * Author: Shradha Todi <shradha.t@samsung.com>
> + */
> +
> +#include <linux/debugfs.h>
> +
> +#include "pcie-designware.h"
> +
> +#define SD_STATUS_L1LANE_REG		0xb0

Sort these definitions.

Rest looks good to me.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support in DWC

[Manivannan Sadhasivam]

On Wed, Dec 11, 2024 at 04:38:34PM +0530, Shradha Todi wrote:
> 
> 
> > -----Original Message-----
> > From: Fan Ni <nifan.cxl@gmail.com>
> > Sent: 10 December 2024 03:59
> > To: Shradha Todi <shradha.t@samsung.com>
> > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> > kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> > a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com; quic_krichai@quicinc.com;
> > gost.dev@samsung.com
> > Subject: Re: [PATCH v4 2/2] PCI: dwc: Add debugfs based RASDES support in DWC
> > 
> > On Fri, Dec 06, 2024 at 01:14:56PM +0530, Shradha Todi wrote:
> > > Add support to use the RASDES feature of DesignWare PCIe controller
> > > using debugfs entries.
> > >
> > > RASDES is a vendor specific extended PCIe capability which reads the
> > > current hardware internal state of PCIe device. Following primary
> > > features are provided to userspace via debugfs:
> > > - Debug registers
> > > - Error injection
> > > - Statistical counters
> > 
> > I think this patch can break into several to make it easier to review.
> > For example, it can be divided by the three features list above, with the documentation change coming last as a
> separate
> > patch.
> > 
> 
> Sure Fan. I have no issues in breaking this into smaller patches. Though I think the documentation
> should go along with the implementation rather than a separate patch?
> Anyway, I'll wait for some time for further review comments or if anyone has any objection to
> splitting the patches before going for the next revision.
> 

I don't mind splitting the patches, but as you said, the documentation should go
in the same patch that adds driver support.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

RE: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Shradha Todi]

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 15 January 2025 22:00
> To: Bjorn Helgaas <helgaas@kernel.org>
> Cc: Shradha Todi <shradha.t@samsung.com>; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> fan.ni@samsung.com; a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> quic_krichai@quicinc.com; gost.dev@samsung.com
> Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> 
> On Wed, Jan 15, 2025 at 10:12:01AM -0600, Bjorn Helgaas wrote:
> > On Wed, Jan 15, 2025 at 08:57:42PM +0530, Manivannan Sadhasivam wrote:
> > > On Wed, Dec 11, 2024 at 08:43:30AM -0600, Bjorn Helgaas wrote:
> > > > On Wed, Dec 11, 2024 at 05:15:50PM +0530, Shradha Todi wrote:
> > > > > > -----Original Message-----
> > > > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > > > > Sent: 06 December 2024 21:43
> > > > > > To: Shradha Todi <shradha.t@samsung.com>
> > > > > > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org;
> > > > > > manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> > > > > > kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> > > > > > jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> > > > > > fan.ni@samsung.com; a.manzanares@samsung.com;
> > > > > > pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> > > > > > quic_krichai@quicinc.com; gost.dev@samsung.com
> > > > > > Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor
> > > > > > specific capability search
> > > > > >
> > > > > > On Fri, Dec 06, 2024 at 01:14:55PM +0530, Shradha Todi wrote:
> > > > > > > Add vendor specific extended configuration space capability
> > > > > > > search API using struct dw_pcie pointer for DW controllers.
> > > > > > >
> > > > > > > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > > > > > > ---
> > > > > > >  drivers/pci/controller/dwc/pcie-designware.c | 16
> > > > > > > ++++++++++++++++
> > > > > > > drivers/pci/controller/dwc/pcie-designware.h |  1 +
> > > > > > >  2 files changed, 17 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > > index 6d6cbc8b5b2c..41230c5e4a53 100644
> > > > > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > > @@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> > > > > > >  	return 0;
> > > > > > >  }
> > > > > > >
> > > > > > > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8
> > > > > > > +vsec_cap)
> > > > > >
> > > > > > To make sure that we find a VSEC ID that corresponds to the
> > > > > > expected vendor, I think this interface needs to be the same
> > > > > > as pci_find_vsec_capability().  In particular, it needs to
> > > > > > take a
> > > > > > "u16 vendor"
> > > > >
> > > > > As per my understanding, Synopsys is the vendor here when we
> > > > > talk about vsec capabilities.  VSEC cap IDs are fixed for each
> > > > > vendor
> > > > > (eg: For Synopsys Designware controllers, 0x2 is always RAS CAP,
> > > > > 0x4 is always PTM responder and so on).
> > > >
> > > > For VSEC, the vendor that matters is the one identified at 0x0 in
> > > > config space.  That's why pci_find_vsec_capability() checks the
> > > > supplied "vendor" against "dev->vendor".
> > > >
> > > > > So no matter if the DWC IP is being integrated by Samsung, NVDIA
> > > > > or Qcom, the vendor specific CAP IDs will remain constant. Now
> > > > > since this function is being written as part of designware file,
> > > > > the control will reach here only when the PCIe IP is DWC. So, we
> > > > > don't really require a vendor ID to be checked here. EG: If 0x2
> > > > > VSEC ID is present in any DWC controller, it means RAS is
> > > > > supported. Please correct me if I'm wrong.
> > > >
> > > > In this case, the Vendor ID is typically Samsung, NVIDIA, Qcom,
> > > > etc., even though it may contain Synopsys DWC IP.  Each vendor
> > > > assigns VSEC IDs independently, so VSEC ID 0x2 may mean something
> > > > different to Samsung than it does to NVIDIA or Qcom.
> > > >
> > > > PCIe r6.0, sec 7.9.5 has the details, but the important part is this:
> > > >
> > > >   With a PCI Express Function, the structure and definition of the
> > > >   vendor-specific Registers area is determined by the vendor indicated
> > > >   by the Vendor ID field located at byte offset 00h in PCI-compatible
> > > >   Configuration Space.
> > > >
> > > > There IS a separate DVSEC ("Designated Vendor-Specific")
> > > > Capability; see sec 7.9.6.  That one does include a DVSEC Vendor
> > > > ID in the Capability itself, and this would make more sense for this situation.
> > > >
> > > > If Synopsys assigned DVSEC ID 0x2 from the Synopsys namespace for
> > > > RAS, then devices from Samsung, NVIDIA, Qcom, etc., could
> > > > advertise a DVSEC Capability that contained a DVSEC Vendor ID of
> > > > PCI_VENDOR_ID_SYNOPSYS with DVSEC ID 0x2, and all those devices could easily locate it.
> > > >
> > > > Unfortunately Samsung et al used VSEC instead of DVSEC, so we're
> > > > stuck with having to specify the device vendor and the VSEC ID
> > > > assigned by that vendor, and those VSEC IDs might be different per vendor.
> > >
> > > Atleast on Qcom platforms, VSEC_ID is 0x2 for RAS. But this is not
> > > guaranteed to be the same as per the PCIe spec as you mentioned.
> > > Though, I think it is safe to go with it since we have seen the same
> > > IDs on 2 platforms (my gut feeling is that it is going to be the
> > > same on other DWC vendor platforms as well). If we encounter
> > > different IDs, then we can add vendor id check.
> >
> > This series uses:
> >
> >   dw_pcie_find_vsec_capability(pci, DW_PCIE_VSEC_EXT_CAP_RAS_DES)
> >
> > in dwc_pcie_rasdes_debugfs_init(), but I don't see any calls of that
> > function yet.
> 
> I guess that the caller got missed unintentionally in patch 2/2.

Actually the missing caller is intentional. Jonathan rightly pointed out in the
previous version that the function : dw_pcie_setup() was being called in the
resume path as well and so I thought it would be best to leave it up to the
platform drivers to decide when and how to call the rasdes init. Do you suggest any
other approach?

> 
> >  If it is called only from code that already knows the device vendor
> > has assigned VSEC ID 0x02 for the DWC RAS functionality, I guess it is
> > "safe".
> >
> 
> It should be called from the DWC code driver (pcie-desginware-host.c).
> 
> > But I think it would be a bad idea because it perpetuates the
> > misunderstanding that DesignWare can independently claim ownership of
> > VSEC ID 0x02 for *all* vendors, and other vendors have already used
> > VSEC ID 0x02 for different things (examples at [1]).  If any of them
> > incorporates this DWC IP, they will have to use a different VSEC ID to
> > avoid a collision with their existing VSEC ID 0x02.
> >
> 
> Fair enough. I was trying to avoid updating the vendor id table for enabling the RAS DES debug feature, but I think it would be worth
> doing so (perf driver is also doing the same).

Makes sense to add the vendor ID check. Will include it in the next version.

> 
> So yeah, I'm OK with the idea of having the vendor_id check in this API.
> 
> (Also, I don't see the VSEC_IDs defined in the DWC reference manual that I got access to).
> 
> - Mani
> 
> --
> மணிவண்ணன் சதாசிவம்

RE: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

[Shradha Todi]

> -----Original Message-----
> From: Shradha Todi <shradha.t@samsung.com>
> Sent: 15 January 2025 22:33
> To: 'Manivannan Sadhasivam' <manivannan.sadhasivam@linaro.org>; 'Bjorn Helgaas' <helgaas@kernel.org>
> Cc: 'linux-kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>; 'linux-pci@vger.kernel.org' <linux-pci@vger.kernel.org>;
> 'lpieralisi@kernel.org' <lpieralisi@kernel.org>; 'kw@linux.com' <kw@linux.com>; 'robh@kernel.org' <robh@kernel.org>;
> 'bhelgaas@google.com' <bhelgaas@google.com>; 'jingoohan1@gmail.com' <jingoohan1@gmail.com>;
> 'Jonathan.Cameron@huawei.com' <Jonathan.Cameron@huawei.com>; 'fan.ni@samsung.com' <fan.ni@samsung.com>;
> 'a.manzanares@samsung.com' <a.manzanares@samsung.com>; 'pankaj.dubey@samsung.com' <pankaj.dubey@samsung.com>;
> 'quic_nitegupt@quicinc.com' <quic_nitegupt@quicinc.com>; 'quic_krichai@quicinc.com' <quic_krichai@quicinc.com>;
> 'gost.dev@samsung.com' <gost.dev@samsung.com>
> Subject: RE: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> 
> 
> 
> > -----Original Message-----
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Sent: 15 January 2025 22:00
> > To: Bjorn Helgaas <helgaas@kernel.org>
> > Cc: Shradha Todi <shradha.t@samsung.com>; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; lpieralisi@kernel.org;
> > kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> > fan.ni@samsung.com; a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> > quic_krichai@quicinc.com; gost.dev@samsung.com
> > Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> >
> > On Wed, Jan 15, 2025 at 10:12:01AM -0600, Bjorn Helgaas wrote:
> > > On Wed, Jan 15, 2025 at 08:57:42PM +0530, Manivannan Sadhasivam wrote:
> > > > On Wed, Dec 11, 2024 at 08:43:30AM -0600, Bjorn Helgaas wrote:
> > > > > On Wed, Dec 11, 2024 at 05:15:50PM +0530, Shradha Todi wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > > > > > Sent: 06 December 2024 21:43
> > > > > > > To: Shradha Todi <shradha.t@samsung.com>
> > > > > > > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org;
> > > > > > > manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> > > > > > > kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> > > > > > > jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> > > > > > > fan.ni@samsung.com; a.manzanares@samsung.com;
> > > > > > > pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> > > > > > > quic_krichai@quicinc.com; gost.dev@samsung.com
> > > > > > > Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor
> > > > > > > specific capability search
> > > > > > >
> > > > > > > On Fri, Dec 06, 2024 at 01:14:55PM +0530, Shradha Todi wrote:
> > > > > > > > Add vendor specific extended configuration space capability
> > > > > > > > search API using struct dw_pcie pointer for DW controllers.
> > > > > > > >
> > > > > > > > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > > > > > > > ---
> > > > > > > >  drivers/pci/controller/dwc/pcie-designware.c | 16
> > > > > > > > ++++++++++++++++
> > > > > > > > drivers/pci/controller/dwc/pcie-designware.h |  1 +
> > > > > > > >  2 files changed, 17 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > > > index 6d6cbc8b5b2c..41230c5e4a53 100644
> > > > > > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > > > @@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> > > > > > > >  	return 0;
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8
> > > > > > > > +vsec_cap)
> > > > > > >
> > > > > > > To make sure that we find a VSEC ID that corresponds to the
> > > > > > > expected vendor, I think this interface needs to be the same
> > > > > > > as pci_find_vsec_capability().  In particular, it needs to
> > > > > > > take a
> > > > > > > "u16 vendor"
> > > > > >
> > > > > > As per my understanding, Synopsys is the vendor here when we
> > > > > > talk about vsec capabilities.  VSEC cap IDs are fixed for each
> > > > > > vendor
> > > > > > (eg: For Synopsys Designware controllers, 0x2 is always RAS CAP,
> > > > > > 0x4 is always PTM responder and so on).
> > > > >
> > > > > For VSEC, the vendor that matters is the one identified at 0x0 in
> > > > > config space.  That's why pci_find_vsec_capability() checks the
> > > > > supplied "vendor" against "dev->vendor".
> > > > >
> > > > > > So no matter if the DWC IP is being integrated by Samsung, NVDIA
> > > > > > or Qcom, the vendor specific CAP IDs will remain constant. Now
> > > > > > since this function is being written as part of designware file,
> > > > > > the control will reach here only when the PCIe IP is DWC. So, we
> > > > > > don't really require a vendor ID to be checked here. EG: If 0x2
> > > > > > VSEC ID is present in any DWC controller, it means RAS is
> > > > > > supported. Please correct me if I'm wrong.
> > > > >
> > > > > In this case, the Vendor ID is typically Samsung, NVIDIA, Qcom,
> > > > > etc., even though it may contain Synopsys DWC IP.  Each vendor
> > > > > assigns VSEC IDs independently, so VSEC ID 0x2 may mean something
> > > > > different to Samsung than it does to NVIDIA or Qcom.
> > > > >
> > > > > PCIe r6.0, sec 7.9.5 has the details, but the important part is this:
> > > > >
> > > > >   With a PCI Express Function, the structure and definition of the
> > > > >   vendor-specific Registers area is determined by the vendor indicated
> > > > >   by the Vendor ID field located at byte offset 00h in PCI-compatible
> > > > >   Configuration Space.
> > > > >
> > > > > There IS a separate DVSEC ("Designated Vendor-Specific")
> > > > > Capability; see sec 7.9.6.  That one does include a DVSEC Vendor
> > > > > ID in the Capability itself, and this would make more sense for this situation.
> > > > >
> > > > > If Synopsys assigned DVSEC ID 0x2 from the Synopsys namespace for
> > > > > RAS, then devices from Samsung, NVIDIA, Qcom, etc., could
> > > > > advertise a DVSEC Capability that contained a DVSEC Vendor ID of
> > > > > PCI_VENDOR_ID_SYNOPSYS with DVSEC ID 0x2, and all those devices could easily locate it.
> > > > >
> > > > > Unfortunately Samsung et al used VSEC instead of DVSEC, so we're
> > > > > stuck with having to specify the device vendor and the VSEC ID
> > > > > assigned by that vendor, and those VSEC IDs might be different per vendor.
> > > >
> > > > Atleast on Qcom platforms, VSEC_ID is 0x2 for RAS. But this is not
> > > > guaranteed to be the same as per the PCIe spec as you mentioned.
> > > > Though, I think it is safe to go with it since we have seen the same
> > > > IDs on 2 platforms (my gut feeling is that it is going to be the
> > > > same on other DWC vendor platforms as well). If we encounter
> > > > different IDs, then we can add vendor id check.
> > >
> > > This series uses:
> > >
> > >   dw_pcie_find_vsec_capability(pci, DW_PCIE_VSEC_EXT_CAP_RAS_DES)
> > >
> > > in dwc_pcie_rasdes_debugfs_init(), but I don't see any calls of that
> > > function yet.
> >
> > I guess that the caller got missed unintentionally in patch 2/2.
> 
> Actually the missing caller is intentional. Jonathan rightly pointed out in the
> previous version that the function : dw_pcie_setup() was being called in the
> resume path as well and so I thought it would be best to leave it up to the
> platform drivers to decide when and how to call the rasdes init. Do you suggest any
> other approach?
> 

On second thoughts, I will add the dwc_pcie_rasdes_debugfs_init and deinit calls in the
dwc common PCIe files but in the probe/remove path.

> >
> > >  If it is called only from code that already knows the device vendor
> > > has assigned VSEC ID 0x02 for the DWC RAS functionality, I guess it is
> > > "safe".
> > >
> >
> > It should be called from the DWC code driver (pcie-desginware-host.c).
> >
> > > But I think it would be a bad idea because it perpetuates the
> > > misunderstanding that DesignWare can independently claim ownership of
> > > VSEC ID 0x02 for *all* vendors, and other vendors have already used
> > > VSEC ID 0x02 for different things (examples at [1]).  If any of them
> > > incorporates this DWC IP, they will have to use a different VSEC ID to
> > > avoid a collision with their existing VSEC ID 0x02.
> > >
> >
> > Fair enough. I was trying to avoid updating the vendor id table for enabling the RAS DES debug feature, but I think it would be worth
> > doing so (perf driver is also doing the same).
> 
> Makes sense to add the vendor ID check. Will include it in the next version.
> 
> >
> > So yeah, I'm OK with the idea of having the vendor_id check in this API.
> >
> > (Also, I don't see the VSEC_IDs defined in the DWC reference manual that I got access to).
> >
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

['Manivannan Sadhasivam']

On Thu, Jan 16, 2025 at 12:42:23PM +0530, Shradha Todi wrote:

[...]

> > > > This series uses:
> > > >
> > > >   dw_pcie_find_vsec_capability(pci, DW_PCIE_VSEC_EXT_CAP_RAS_DES)
> > > >
> > > > in dwc_pcie_rasdes_debugfs_init(), but I don't see any calls of that
> > > > function yet.
> > >
> > > I guess that the caller got missed unintentionally in patch 2/2.
> > 
> > Actually the missing caller is intentional. Jonathan rightly pointed out in the
> > previous version that the function : dw_pcie_setup() was being called in the
> > resume path as well and so I thought it would be best to leave it up to the
> > platform drivers to decide when and how to call the rasdes init. Do you suggest any
> > other approach?
> > 

Adding the API without any in-kernel consumer is not usually recommended.

> 
> On second thoughts, I will add the dwc_pcie_rasdes_debugfs_init and deinit calls in the
> dwc common PCIe files but in the probe/remove path.
> 

Can you please be more specific? There are no probe/remove functions in DWC
common drivers. We have init/deinit only. For pcie-designware-host, you can call
dwc_pcie_rasdes_debugfs_init() from dw_pcie_host_init() and
dwc_pcie_rasdes_debugfs_deinit() from dw_pcie_host_deinit(). But for
pcie-designware-ep, you should call them from dw_pcie_ep_init_registers() and
dw_pcie_ep_cleanup() since reading/writing to these debugfs files will cause
DBI access and that requires active refclk. These APIs are used as a placeholder
for code that require refclk to work.

- Mani

-- 
மணிவண்ணன் சதாசிவம்