On 26-Sep-18 7:18 PM, Andy Shevchenko wrote:
On Mon, Sep 3, 2018 at 9:06 PM Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxxxxxxxxxx> wrote:This adds support to show the Latency Tolerance Reporting for the IPs on the PCH as reported by the PMC. The format shown here is raw LTR data payload that can further be decoded as per the PCI specification. This also fixes some minor alignment issues in the header file by removing spaces and converting to tabs at some places.Thanks for the patch, my comments below.
Hi Andy, Thanks for the review, my answers below.
+static const struct pmc_bit_map spt_ltr_show_map[] = { + {"IP 0 : LTR_SOUTHPORT_A", SPT_PMC_LTR_SPA}, + {"IP 1 : LTR_SOUTHPORT_B", SPT_PMC_LTR_SPB}, + {"IP 2 : LTR_SATA", SPT_PMC_LTR_SATA}, + {"IP 3 : LTR_GIGABIT_ETHERNET", SPT_PMC_LTR_GBE}, + {"IP 4 : LTR_XHCI", SPT_PMC_LTR_XHCI}, + /* IP 5 is reserved */ + {"IP 6 : LTR_ME", SPT_PMC_LTR_ME}, + /* EVA is Enterprise Value Add, doesn't really exist on PCH */ + {"IP 7 : LTR_EVA", SPT_PMC_LTR_EVA}, + {"IP 8 : LTR_SOUTHPORT_C", SPT_PMC_LTR_SPC}, + {"IP 9 : LTR_HD_AUDIO", SPT_PMC_LTR_AZ}, + /* IP 10 is reserved */ + {"IP 11 : LTR_LPSS", SPT_PMC_LTR_LPSS}, + {"IP 12 : LTR_SOUTHPORT_D", SPT_PMC_LTR_SPD}, + {"IP 13 : LTR_SOUTHPORT_E", SPT_PMC_LTR_SPE}, + {"IP 14 : LTR_CAMERA", SPT_PMC_LTR_CAM}, + {"IP 15 : LTR_ESPI", SPT_PMC_LTR_ESPI}, + {"IP 16 : LTR_SCC", SPT_PMC_LTR_SCC}, + {"IP 17 : LTR_ISH", SPT_PMC_LTR_ISH}, + /* Below two cannot be for LTR_IGNORE */ + {"LTR_CURRENT_PLATFORM", SPT_PMC_LTR_CUR_PLT}, + {"LTR_AGGREGATED_SYSTEM", SPT_PMC_LTR_CUR_ASLT},Before no map has this fancy "IP xx :" prefixes. Please, remove.
The users of the driver often ask for IP Numbers while performing LTR_IGNORE operation so this is deliberately added. Please consider it.
+ {},No need for comma
Ok.
+Redundant.
OK
+}; +static const struct pmc_bit_map cnp_ltr_show_map[] = {Same comments as above.+}; + debugfs_create_file("ltr_show", 0644, dir, pmcdev, + &pmc_core_ltr_fops);One line?
IIRC, it was crossing the limit. I will check again and if possible would change it.
#define NUM_RETRIES 100 #define NUM_IP_IGN_ALLOWED 17+ blank line here.
Sure.
+#define SPT_PMC_LTR_CUR_PLT 0x350
