linux-kernel - Re: [PATCH v2 17/17] PCI: qcom: Expose link transition counts via debugfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <73269ad0-5662-519c-f6c0-e5e2f918cd4e@quicinc.com>
Date:   Thu, 9 Mar 2023 17:21:38 +0530
From:   Sricharan Ramabadhran <quic_srichara@...cinc.com>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        <andersson@...nel.org>, <lpieralisi@...nel.org>, <kw@...ux.com>,
        <krzysztof.kozlowski+dt@...aro.org>, <robh@...nel.org>
CC:     <konrad.dybcio@...aro.org>, <linux-arm-msm@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-pci@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 17/17] PCI: qcom: Expose link transition counts via
 debugfs



On 3/9/2023 2:21 PM, Manivannan Sadhasivam wrote:
> Qualcomm PCIe controllers have debug registers in the MHI region that
> count PCIe link transitions. Expose them over debugfs to userspace to
> help debug the low power issues.
> 
> Note that even though the registers are prefixed as PARF_, they don't
> live under the "parf" register region. The register naming is following
> the Qualcomm's internal documentation as like other registers.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> ---
>   drivers/pci/controller/dwc/pcie-qcom.c | 59 ++++++++++++++++++++++++++
>   1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index e1180c84f0fa..6d9bde64c9e9 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -10,6 +10,7 @@
>   
>   #include <linux/clk.h>
>   #include <linux/crc8.h>
> +#include <linux/debugfs.h>
>   #include <linux/delay.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/interconnect.h>
> @@ -62,6 +63,13 @@
>   #define AXI_MSTR_RESP_COMP_CTRL1		0x81c
>   #define MISC_CONTROL_1_REG			0x8bc
>   
> +/* MHI registers */
> +#define PARF_DEBUG_CNT_PM_LINKST_IN_L2		0xc04
> +#define PARF_DEBUG_CNT_PM_LINKST_IN_L1		0xc0c
> +#define PARF_DEBUG_CNT_PM_LINKST_IN_L0S		0xc10
> +#define PARF_DEBUG_CNT_AUX_CLK_IN_L1SUB_L1	0xc84
> +#define PARF_DEBUG_CNT_AUX_CLK_IN_L1SUB_L2	0xc88
> +
>   /* PARF_SYS_CTRL register fields */
>   #define MAC_PHY_POWERDOWN_IN_P2_D_MUX_EN	BIT(29)
>   #define MST_WAKEUP_EN				BIT(13)
> @@ -229,11 +237,13 @@ struct qcom_pcie {
>   	struct dw_pcie *pci;
>   	void __iomem *parf;			/* DT parf */
>   	void __iomem *elbi;			/* DT elbi */
> +	void __iomem *mhi;
>   	union qcom_pcie_resources res;
>   	struct phy *phy;
>   	struct gpio_desc *reset;
>   	struct icc_path *icc_mem;
>   	const struct qcom_pcie_cfg *cfg;
> +	struct dentry *debugfs;
>   };
>   
>   #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> @@ -1385,6 +1395,37 @@ static void qcom_pcie_icc_update(struct qcom_pcie *pcie)
>   	}
>   }
>   
> +static int qcom_pcie_link_transition_count(struct seq_file *s, void *data)
> +{
> +	struct qcom_pcie *pcie = (struct qcom_pcie *)
> +				     dev_get_drvdata(s->private);
> +
> +	seq_printf(s, "L0s transition count: %u\n",
> +		   readl_relaxed(pcie->mhi + PARF_DEBUG_CNT_PM_LINKST_IN_L0S));
> +
> +	seq_printf(s, "L1 transition count: %u\n",
> +		   readl_relaxed(pcie->mhi + PARF_DEBUG_CNT_PM_LINKST_IN_L1));
> +
> +	seq_printf(s, "L1.1 transition count: %u\n",
> +		   readl_relaxed(pcie->mhi + PARF_DEBUG_CNT_AUX_CLK_IN_L1SUB_L1));
> +
> +	seq_printf(s, "L1.2 transition count: %u\n",
> +		   readl_relaxed(pcie->mhi + PARF_DEBUG_CNT_AUX_CLK_IN_L1SUB_L2));
> +
> +	seq_printf(s, "L2 transition count: %u\n",
> +		   readl_relaxed(pcie->mhi + PARF_DEBUG_CNT_PM_LINKST_IN_L2));
> +
> +	return 0;
> +}
> +
> +static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie)
> +{
> +	struct dw_pcie *pci = pcie->pci;
> +
> +	debugfs_create_devm_seqfile(pci->dev, "link_transition_count", pcie->debugfs,
> +				    qcom_pcie_link_transition_count);
> +}
> +
>   static int qcom_pcie_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> @@ -1392,6 +1433,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>   	struct dw_pcie *pci;
>   	struct qcom_pcie *pcie;
>   	const struct qcom_pcie_cfg *pcie_cfg;
> +	char *name;
>   	int ret;
>   
>   	pcie_cfg = of_device_get_match_data(dev);
> @@ -1439,6 +1481,12 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>   		goto err_pm_runtime_put;
>   	}
>   
> +	pcie->mhi = devm_platform_ioremap_resource_byname(pdev, "mhi");
> +	if (IS_ERR(pcie->mhi)) {
> +		ret = PTR_ERR(pcie->mhi);
> +		goto err_pm_runtime_put;
> +	}
> +

  Tested this series on ipq4019-ap.dk07.1-c1 board and the above hunk
  breaks enumeration because there is no 'mhi' region. All the debug bits
  used in the transition_count function is inside the PARF_STTS register
  at offset 0x24 inside the PARF region.

  Register: PCIE_0_PCIE20_PARF_PM_STTS | 0x80024
  Offset:    0x24    Reset State:    0x00040000

Bits    Field Name
31    LINK_REQ_RST_NOT
30    XMLH_LINK_UP
29    PM_DSTATE_0
0x0: D0
0x1: D3
28    PHYSTATUS
27:16    PM_DSTATE
15:12    PM_PME_EN
11    PHYCLK_REQ_N
10    L1SS_CLKREQN_OE
9    L1SS_CLKREQN_IN
8    PM_LINKST_IN_L1SUB
7    PM_LINKST_IN_L0S
6    PM_LINKST_L2_EXIT
5    PM_LINKST_IN_L2
4    PM_LINKST_IN_L1
3:0    PM_STATUS

Otherwise, with rest of the patches enumeration was fine.
Tested with a pcie ethernet adapter.

Regards,
  Sricharan