| 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
| ||
|
Message-ID: <20230309145328.GD5151@thinkpad>
Date: Thu, 9 Mar 2023 20:23:28 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Sricharan Ramabadhran <quic_srichara@...cinc.com>
Cc: andersson@...nel.org, lpieralisi@...nel.org, kw@...ux.com,
krzysztof.kozlowski+dt@...aro.org, robh@...nel.org,
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 Thu, Mar 09, 2023 at 05:21:38PM +0530, Sricharan Ramabadhran wrote:
>
>
> 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.
>
Ah, "mhi" is supposed to be optional, my bad. Will fix it in next revision.
> Register: PCIE_0_PCIE20_PARF_PM_STTS | 0x80024
> Offset: 0x24 Reset State: 0x00040000
>
Hmm, is this register present on other IPQ/APQ SoCs?
> 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.
>
Thanks a lot for testing!
- Mani
> Regards,
> Sricharan
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists