Re: [PATCH v3 4/8] hisi_ptt: Add tune function support for HiSilicon PCIe Tune and Trace device
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Date: 2022-02-07 13:16:54
Also in:
linux-iommu, linux-pci, linux-perf-users, lkml
On Mon, 24 Jan 2022 21:11:14 +0800 Yicong Yang [off-list ref] wrote:
Add tune function for the HiSilicon Tune and Trace device. The interface of tune is exposed through sysfs attributes of PTT PMU device. Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
A few trivial things inline, but looks good in general to me. With those tidied up Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
quoted hunk ↗ jump to hunk
--- drivers/hwtracing/ptt/hisi_ptt.c | 154 +++++++++++++++++++++++++++++++ drivers/hwtracing/ptt/hisi_ptt.h | 19 ++++ 2 files changed, 173 insertions(+)diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c index 2994354e690b..b11e702eb506 100644 --- a/drivers/hwtracing/ptt/hisi_ptt.c +++ b/drivers/hwtracing/ptt/hisi_ptt.c@@ -21,6 +21,159 @@ #include "hisi_ptt.h" +static int hisi_ptt_wait_tuning_finish(struct hisi_ptt *hisi_ptt) +{ + u32 val; + + return readl_poll_timeout(hisi_ptt->iobase + HISI_PTT_TUNING_INT_STAT, + val, !(val & HISI_PTT_TUNING_INT_STAT_MASK), + HISI_PTT_WAIT_POLL_INTERVAL_US, + HISI_PTT_WAIT_TIMEOUT_US); +} + +static int hisi_ptt_tune_data_get(struct hisi_ptt *hisi_ptt, + u32 event, u16 *data) +{ + u32 reg; + + reg = readl(hisi_ptt->iobase + HISI_PTT_TUNING_CTRL); + reg &= ~(HISI_PTT_TUNING_CTRL_CODE | HISI_PTT_TUNING_CTRL_SUB); + reg |= FIELD_PREP(HISI_PTT_TUNING_CTRL_CODE | HISI_PTT_TUNING_CTRL_SUB, + event); + writel(reg, hisi_ptt->iobase + HISI_PTT_TUNING_CTRL); + + /* Write all 1 to indicates it's the read process */ + writel(~0UL, hisi_ptt->iobase + HISI_PTT_TUNING_DATA);
Just to check, this is includes the bits above the DATA_VAL_MASK? Fine if so, just seems odd to define a field but then write parts of the register that aren't part of that field.
+ + if (hisi_ptt_wait_tuning_finish(hisi_ptt)) + return -ETIMEDOUT; + + reg = readl(hisi_ptt->iobase + HISI_PTT_TUNING_DATA); + reg &= HISI_PTT_TUNING_DATA_VAL_MASK; + *data = (u16)reg;
As below, prefer a FIELD_GET() for this.
+
+ return 0;
+}
+
+static int hisi_ptt_tune_data_set(struct hisi_ptt *hisi_ptt,
+ u32 event, u16 data)
+{
+ u32 reg;
+
+ reg = readl(hisi_ptt->iobase + HISI_PTT_TUNING_CTRL);
+ reg &= ~(HISI_PTT_TUNING_CTRL_CODE | HISI_PTT_TUNING_CTRL_SUB);
+ reg |= FIELD_PREP(HISI_PTT_TUNING_CTRL_CODE | HISI_PTT_TUNING_CTRL_SUB,
+ event);
+ writel(reg, hisi_ptt->iobase + HISI_PTT_TUNING_CTRL);
+
+ reg = data;Given you defined HISI_PTT_TUNING_DATA_VAL_MASK why not use it here writel(FIELD_PREP(..), ...)?
+ writel(reg, hisi_ptt->iobase + HISI_PTT_TUNING_DATA); + + if (hisi_ptt_wait_tuning_finish(hisi_ptt)) + return -ETIMEDOUT; + + return 0; +} +
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel