Re: [PATCH v5 2/4] Add debugfs based silicon debug support in DWC

[Niklas Cassel <cassel@kernel.org>]

Hello Shradha,

On Tue, Jan 21, 2025 at 04:44:19PM +0530, Shradha Todi wrote:

This is the suggested format of your new feature:

> +
> +struct dwc_pcie_vendor_id {
> +	u16 vendor_id;
> +	u16 vsec_rasdes_cap_id;
> +};
> +
> +static const struct dwc_pcie_vendor_id dwc_pcie_vendor_ids[] = {
> +	{PCI_VENDOR_ID_SAMSUNG,	0x2},
> +	{} /* terminator */
> +};


You might know of the drivers/perf driver which also exposes RAS information
for DWC. That driver uses the following format for supported entries:

+struct dwc_pcie_pmu_vsec_id {
+	u16 vendor_id;
+	u16 vsec_id;
+	u8 vsec_rev;
 };

+/*
+ * VSEC IDs are allocated by the vendor, so a given ID may mean different
+ * things to different vendors.  See PCIe r6.0, sec 7.9.5.2.
+ */
+static const struct dwc_pcie_pmu_vsec_id dwc_pcie_pmu_vsec_ids[] = {
+	{ .vendor_id = PCI_VENDOR_ID_ALIBABA,
+	  .vsec_id = 0x02, .vsec_rev = 0x4 },
+	{ .vendor_id = PCI_VENDOR_ID_AMPERE,
+	  .vsec_id = 0x02, .vsec_rev = 0x4 },
+	{ .vendor_id = PCI_VENDOR_ID_QCOM,
+	  .vsec_id = 0x02, .vsec_rev = 0x4 },
 	{} /* terminator */
 };

See:
https://lore.kernel.org/all/20241209222938.3219364-1-helgaas@kernel.org/


I think it would be a good idea for your feature to use the exact same
format for supported entries, so that entries can simply be copy pasted
between the two drivers.

(Considering that both of these drivers are simply exposing the RAS
information in different ways, having an entry in one of the two drivers
should mean that that entry should work/be applicable for the other
driver as well.)


You might also want to add support for Samsung in the drivers/perf driver.


Kind regards,
Niklas