2024-02-09 08:40:51

by Shravan Ramani

[permalink] [raw]
Subject: [PATCH v2 0/4] Updates to mlxbf-pmc

This submission contains 4 patches:
(N) Patch 1 replaces all uintN_t usage with kernel-style types
(N) Patch 2 resolves signed/unsigned int mix-up
Patch 3 adds support for 64-bit counters and tracking cycle count
Patch 4 adds support for the clock_measure performance block

v1 -> v2
Added 2 new patches to address generic issues
Replaced all uintN usage in the driver
Fixed signed/unsigned mix-up and replaced identifiers accordingly
Replaced kstrtoint with kstrtouint as applicable
Retained devm_kasprintf usage since other instances require dynamic allocation

Shravan Kumar Ramani (4):
platform/mellanox: mlxbf-pmc: Replace uintN_t with kernel-style types
platform/mellanox: mlxbf-pmc: Fix signed/unsigned mix-up
platform/mellanox: mlxbf-pmc: Add support for 64-bit counters and
cycle count
platform/mellanox: mlxbf-pmc: Add support for clock_measure
performance block

drivers/platform/mellanox/mlxbf-pmc.c | 381 +++++++++++++++++++-------
1 file changed, 278 insertions(+), 103 deletions(-)

--
2.30.1



2024-02-09 08:41:08

by Shravan Ramani

[permalink] [raw]
Subject: [PATCH v2 1/4] platform/mellanox: mlxbf-pmc: Replace uintN_t with kernel-style types

Signed-off-by: Shravan Kumar Ramani <[email protected]>
---
drivers/platform/mellanox/mlxbf-pmc.c | 121 +++++++++++++-------------
1 file changed, 59 insertions(+), 62 deletions(-)

diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
index b1995ac268d7..71d919832e2a 100644
--- a/drivers/platform/mellanox/mlxbf-pmc.c
+++ b/drivers/platform/mellanox/mlxbf-pmc.c
@@ -149,17 +149,17 @@ struct mlxbf_pmc_block_info {
*/
struct mlxbf_pmc_context {
struct platform_device *pdev;
- uint32_t total_blocks;
- uint32_t tile_count;
- uint8_t llt_enable;
- uint8_t mss_enable;
- uint32_t group_num;
+ u32 total_blocks;
+ u32 tile_count;
+ u8 llt_enable;
+ u8 mss_enable;
+ u32 group_num;
struct device *hwmon_dev;
const char *block_name[MLXBF_PMC_MAX_BLOCKS];
struct mlxbf_pmc_block_info block[MLXBF_PMC_MAX_BLOCKS];
const struct attribute_group *groups[MLXBF_PMC_MAX_BLOCKS];
bool svc_sreg_support;
- uint32_t sreg_tbl_perf;
+ u32 sreg_tbl_perf;
unsigned int event_set;
};

@@ -865,8 +865,8 @@ static struct mlxbf_pmc_context *pmc;
static const char *mlxbf_pmc_svc_uuid_str = "89c036b4-e7d7-11e6-8797-001aca00bfc4";

/* Calls an SMC to access a performance register */
-static int mlxbf_pmc_secure_read(void __iomem *addr, uint32_t command,
- uint64_t *result)
+static int mlxbf_pmc_secure_read(void __iomem *addr, u32 command,
+ u64 *result)
{
struct arm_smccc_res res;
int status, err = 0;
@@ -892,8 +892,8 @@ static int mlxbf_pmc_secure_read(void __iomem *addr, uint32_t command,
}

/* Read from a performance counter */
-static int mlxbf_pmc_read(void __iomem *addr, uint32_t command,
- uint64_t *result)
+static int mlxbf_pmc_read(void __iomem *addr, u32 command,
+ u64 *result)
{
if (pmc->svc_sreg_support)
return mlxbf_pmc_secure_read(addr, command, result);
@@ -907,22 +907,22 @@ static int mlxbf_pmc_read(void __iomem *addr, uint32_t command,
}

/* Convenience function for 32-bit reads */
-static int mlxbf_pmc_readl(void __iomem *addr, uint32_t *result)
+static int mlxbf_pmc_readl(void __iomem *addr, u32 *result)
{
- uint64_t read_out;
+ u64 read_out;
int status;

status = mlxbf_pmc_read(addr, MLXBF_PMC_READ_REG_32, &read_out);
if (status)
return status;
- *result = (uint32_t)read_out;
+ *result = (u32)read_out;

return 0;
}

/* Calls an SMC to access a performance register */
-static int mlxbf_pmc_secure_write(void __iomem *addr, uint32_t command,
- uint64_t value)
+static int mlxbf_pmc_secure_write(void __iomem *addr, u32 command,
+ u64 value)
{
struct arm_smccc_res res;
int status, err = 0;
@@ -945,7 +945,7 @@ static int mlxbf_pmc_secure_write(void __iomem *addr, uint32_t command,
}

/* Write to a performance counter */
-static int mlxbf_pmc_write(void __iomem *addr, int command, uint64_t value)
+static int mlxbf_pmc_write(void __iomem *addr, int command, u64 value)
{
if (pmc->svc_sreg_support)
return mlxbf_pmc_secure_write(addr, command, value);
@@ -959,7 +959,7 @@ static int mlxbf_pmc_write(void __iomem *addr, int command, uint64_t value)
}

/* Check if the register offset is within the mapped region for the block */
-static bool mlxbf_pmc_valid_range(int blk_num, uint32_t offset)
+static bool mlxbf_pmc_valid_range(int blk_num, u32 offset)
{
if ((offset >= 0) && !(offset % MLXBF_PMC_REG_SIZE) &&
(offset + MLXBF_PMC_REG_SIZE <= pmc->block[blk_num].blk_size))
@@ -1082,7 +1082,7 @@ static char *mlxbf_pmc_get_event_name(const char *blk, int evt)
/* Method to enable/disable/reset l3cache counters */
static int mlxbf_pmc_config_l3_counters(int blk_num, bool enable, bool reset)
{
- uint32_t perfcnt_cfg = 0;
+ u32 perfcnt_cfg = 0;

if (enable)
perfcnt_cfg |= MLXBF_PMC_L3C_PERF_CNT_CFG_EN;
@@ -1095,12 +1095,10 @@ static int mlxbf_pmc_config_l3_counters(int blk_num, bool enable, bool reset)
}

/* Method to handle l3cache counter programming */
-static int mlxbf_pmc_program_l3_counter(int blk_num, uint32_t cnt_num,
- uint32_t evt)
+static int mlxbf_pmc_program_l3_counter(int blk_num, u32 cnt_num,
+ u32 evt)
{
- uint32_t perfcnt_sel_1 = 0;
- uint32_t perfcnt_sel = 0;
- uint32_t *wordaddr;
+ u32 perfcnt_sel_1 = 0, perfcnt_sel = 0, *wordaddr;
void __iomem *pmcaddr;
int ret;

@@ -1162,11 +1160,11 @@ static int mlxbf_pmc_program_l3_counter(int blk_num, uint32_t cnt_num,
}

/* Method to handle crspace counter programming */
-static int mlxbf_pmc_program_crspace_counter(int blk_num, uint32_t cnt_num,
- uint32_t evt)
+static int mlxbf_pmc_program_crspace_counter(int blk_num, u32 cnt_num,
+ u32 evt)
{
- uint32_t word;
void *addr;
+ u32 word;
int ret;

addr = pmc->block[blk_num].mmio_base +
@@ -1187,7 +1185,7 @@ static int mlxbf_pmc_program_crspace_counter(int blk_num, uint32_t cnt_num,
}

/* Method to clear crspace counter value */
-static int mlxbf_pmc_clear_crspace_counter(int blk_num, uint32_t cnt_num)
+static int mlxbf_pmc_clear_crspace_counter(int blk_num, u32 cnt_num)
{
void *addr;

@@ -1199,10 +1197,10 @@ static int mlxbf_pmc_clear_crspace_counter(int blk_num, uint32_t cnt_num)
}

/* Method to program a counter to monitor an event */
-static int mlxbf_pmc_program_counter(int blk_num, uint32_t cnt_num,
- uint32_t evt, bool is_l3)
+static int mlxbf_pmc_program_counter(int blk_num, u32 cnt_num,
+ u32 evt, bool is_l3)
{
- uint64_t perfctl, perfevt, perfmon_cfg;
+ u64 perfctl, perfevt, perfmon_cfg;

if (cnt_num >= pmc->block[blk_num].counters)
return -ENODEV;
@@ -1263,12 +1261,12 @@ static int mlxbf_pmc_program_counter(int blk_num, uint32_t cnt_num,
}

/* Method to handle l3 counter reads */
-static int mlxbf_pmc_read_l3_counter(int blk_num, uint32_t cnt_num,
- uint64_t *result)
+static int mlxbf_pmc_read_l3_counter(int blk_num, u32 cnt_num,
+ u64 *result)
{
- uint32_t perfcnt_low = 0, perfcnt_high = 0;
- uint64_t value;
+ u32 perfcnt_low = 0, perfcnt_high = 0;
int status;
+ u64 value;

status = mlxbf_pmc_readl(pmc->block[blk_num].mmio_base +
MLXBF_PMC_L3C_PERF_CNT_LOW +
@@ -1295,11 +1293,11 @@ static int mlxbf_pmc_read_l3_counter(int blk_num, uint32_t cnt_num,
}

/* Method to handle crspace counter reads */
-static int mlxbf_pmc_read_crspace_counter(int blk_num, uint32_t cnt_num,
- uint64_t *result)
+static int mlxbf_pmc_read_crspace_counter(int blk_num, u32 cnt_num,
+ u64 *result)
{
- uint32_t value;
int status = 0;
+ u32 value;

status = mlxbf_pmc_readl(pmc->block[blk_num].mmio_base +
MLXBF_PMC_CRSPACE_PERFMON_VAL0(pmc->block[blk_num].counters) +
@@ -1313,11 +1311,11 @@ static int mlxbf_pmc_read_crspace_counter(int blk_num, uint32_t cnt_num,
}

/* Method to read the counter value */
-static int mlxbf_pmc_read_counter(int blk_num, uint32_t cnt_num, bool is_l3,
- uint64_t *result)
+static int mlxbf_pmc_read_counter(int blk_num, u32 cnt_num, bool is_l3,
+ u64 *result)
{
- uint32_t perfcfg_offset, perfval_offset;
- uint64_t perfmon_cfg;
+ u32 perfcfg_offset, perfval_offset;
+ u64 perfmon_cfg;
int status;

if (cnt_num >= pmc->block[blk_num].counters)
@@ -1351,13 +1349,12 @@ static int mlxbf_pmc_read_counter(int blk_num, uint32_t cnt_num, bool is_l3,
}

/* Method to read L3 block event */
-static int mlxbf_pmc_read_l3_event(int blk_num, uint32_t cnt_num,
- uint64_t *result)
+static int mlxbf_pmc_read_l3_event(int blk_num, u32 cnt_num,
+ u64 *result)
{
- uint32_t perfcnt_sel = 0, perfcnt_sel_1 = 0;
- uint32_t *wordaddr;
+ u32 perfcnt_sel = 0, perfcnt_sel_1 = 0, *wordaddr;
void __iomem *pmcaddr;
- uint64_t evt;
+ u64 evt;

/* Select appropriate register information */
switch (cnt_num) {
@@ -1405,10 +1402,10 @@ static int mlxbf_pmc_read_l3_event(int blk_num, uint32_t cnt_num,
}

/* Method to read crspace block event */
-static int mlxbf_pmc_read_crspace_event(int blk_num, uint32_t cnt_num,
- uint64_t *result)
+static int mlxbf_pmc_read_crspace_event(int blk_num, u32 cnt_num,
+ u64 *result)
{
- uint32_t word, evt;
+ u32 word, evt;
void *addr;
int ret;

@@ -1429,11 +1426,11 @@ static int mlxbf_pmc_read_crspace_event(int blk_num, uint32_t cnt_num,
}

/* Method to find the event currently being monitored by a counter */
-static int mlxbf_pmc_read_event(int blk_num, uint32_t cnt_num, bool is_l3,
- uint64_t *result)
+static int mlxbf_pmc_read_event(int blk_num, u32 cnt_num, bool is_l3,
+ u64 *result)
{
- uint32_t perfcfg_offset, perfval_offset;
- uint64_t perfmon_cfg, perfevt;
+ u32 perfcfg_offset, perfval_offset;
+ u64 perfmon_cfg, perfevt;

if (cnt_num >= pmc->block[blk_num].counters)
return -EINVAL;
@@ -1469,9 +1466,9 @@ static int mlxbf_pmc_read_event(int blk_num, uint32_t cnt_num, bool is_l3,
}

/* Method to read a register */
-static int mlxbf_pmc_read_reg(int blk_num, uint32_t offset, uint64_t *result)
+static int mlxbf_pmc_read_reg(int blk_num, u32 offset, u64 *result)
{
- uint32_t ecc_out;
+ u32 ecc_out;

if (strstr(pmc->block_name[blk_num], "ecc")) {
if (mlxbf_pmc_readl(pmc->block[blk_num].mmio_base + offset,
@@ -1490,7 +1487,7 @@ static int mlxbf_pmc_read_reg(int blk_num, uint32_t offset, uint64_t *result)
}

/* Method to write to a register */
-static int mlxbf_pmc_write_reg(int blk_num, uint32_t offset, uint64_t data)
+static int mlxbf_pmc_write_reg(int blk_num, u32 offset, u64 data)
{
if (strstr(pmc->block_name[blk_num], "ecc")) {
return mlxbf_pmc_write(pmc->block[blk_num].mmio_base + offset,
@@ -1512,7 +1509,7 @@ static ssize_t mlxbf_pmc_counter_show(struct device *dev,
attr, struct mlxbf_pmc_attribute, dev_attr);
int blk_num, cnt_num, offset;
bool is_l3 = false;
- uint64_t value;
+ u64 value;

blk_num = attr_counter->nr;
cnt_num = attr_counter->index;
@@ -1546,7 +1543,7 @@ static ssize_t mlxbf_pmc_counter_store(struct device *dev,
attr, struct mlxbf_pmc_attribute, dev_attr);
int blk_num, cnt_num, offset, err, data;
bool is_l3 = false;
- uint64_t evt_num;
+ u64 evt_num;

blk_num = attr_counter->nr;
cnt_num = attr_counter->index;
@@ -1597,7 +1594,7 @@ static ssize_t mlxbf_pmc_event_show(struct device *dev,
attr, struct mlxbf_pmc_attribute, dev_attr);
int blk_num, cnt_num, err;
bool is_l3 = false;
- uint64_t evt_num;
+ u64 evt_num;
char *evt_name;

blk_num = attr_event->nr;
@@ -1686,7 +1683,7 @@ static ssize_t mlxbf_pmc_enable_show(struct device *dev,
{
struct mlxbf_pmc_attribute *attr_enable = container_of(
attr, struct mlxbf_pmc_attribute, dev_attr);
- uint32_t perfcnt_cfg, word;
+ u32 perfcnt_cfg, word;
int blk_num, value;

blk_num = attr_enable->nr;
@@ -1718,7 +1715,7 @@ static ssize_t mlxbf_pmc_enable_store(struct device *dev,
struct mlxbf_pmc_attribute *attr_enable = container_of(
attr, struct mlxbf_pmc_attribute, dev_attr);
int err, en, blk_num;
- uint32_t word;
+ u32 word;

blk_num = attr_enable->nr;

@@ -1914,7 +1911,7 @@ static bool mlxbf_pmc_guid_match(const guid_t *guid,
/* Helper to map the Performance Counters from the varios blocks */
static int mlxbf_pmc_map_counters(struct device *dev)
{
- uint64_t info[MLXBF_PMC_INFO_SZ];
+ u64 info[MLXBF_PMC_INFO_SZ];
int i, tile_num, ret;

for (i = 0; i < pmc->total_blocks; ++i) {
--
2.30.1


2024-02-09 08:41:29

by Shravan Ramani

[permalink] [raw]
Subject: [PATCH v2 3/4] platform/mellanox: mlxbf-pmc: Add support for 64-bit counters and cycle count

Add support for programming any counter to monitor the cycle count.
Since counting of cycles using 32-bit ocunters would result in frequent
wraparounds, add the ability to combine 2 adjacent 32-bit counters to
form 1 64-bit counter.
Both these features are supported by BlueField-3 PMC hardware, hence
the required bit-fields are exposed by the driver via sysfs to allow
the user to configure as needed.

Signed-off-by: Shravan Kumar Ramani <[email protected]>
Reviewed-by: David Thompson <[email protected]>
Reviewed-by: Vadim Pasternak <[email protected]>
---
drivers/platform/mellanox/mlxbf-pmc.c | 134 ++++++++++++++++++++++++++
1 file changed, 134 insertions(+)

diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
index e3f1ae772e43..1b9356a3ffab 100644
--- a/drivers/platform/mellanox/mlxbf-pmc.c
+++ b/drivers/platform/mellanox/mlxbf-pmc.c
@@ -88,6 +88,8 @@
#define MLXBF_PMC_CRSPACE_PERFMON_CTL(n) (n * MLXBF_PMC_CRSPACE_PERFMON_REG0_SZ)
#define MLXBF_PMC_CRSPACE_PERFMON_EN BIT(30)
#define MLXBF_PMC_CRSPACE_PERFMON_CLR BIT(28)
+#define MLXBF_PMC_CRSPACE_PERFMON_UOC GENMASK(15, 0)
+#define MLXBF_PMC_CRSPACE_PERFMON_COUNT_CLOCK(n) (MLXBF_PMC_CRSPACE_PERFMON_CTL(n) + 0x4)
#define MLXBF_PMC_CRSPACE_PERFMON_VAL0(n) (MLXBF_PMC_CRSPACE_PERFMON_CTL(n) + 0xc)

/**
@@ -114,6 +116,8 @@ struct mlxbf_pmc_attribute {
* @attr_event: Attributes for "event" sysfs files
* @attr_event_list: Attributes for "event_list" sysfs files
* @attr_enable: Attributes for "enable" sysfs files
+ * @attr_use_odd_counter: Attributes for "use_odd_counter" sysfs files
+ * @attr_count_clock: Attributes for "count_clock" sysfs files
* @block_attr: All attributes needed for the block
* @block_attr_grp: Attribute group for the block
*/
@@ -126,6 +130,8 @@ struct mlxbf_pmc_block_info {
struct mlxbf_pmc_attribute *attr_event;
struct mlxbf_pmc_attribute attr_event_list;
struct mlxbf_pmc_attribute attr_enable;
+ struct mlxbf_pmc_attribute attr_use_odd_counter;
+ struct mlxbf_pmc_attribute attr_count_clock;
struct attribute *block_attr[MLXBF_PMC_MAX_ATTRS];
struct attribute_group block_attr_grp;
};
@@ -1761,6 +1767,103 @@ static ssize_t mlxbf_pmc_enable_store(struct device *dev,
return count;
}

+/* Show function for "use_odd_counter" sysfs files - only for crspace */
+static ssize_t mlxbf_pmc_use_odd_counter_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct mlxbf_pmc_attribute *attr_use_odd_counter = container_of(
+ attr, struct mlxbf_pmc_attribute, dev_attr);
+ unsigned int blk_num;
+ u32 value, reg;
+
+ blk_num = attr_use_odd_counter->nr;
+
+ if (mlxbf_pmc_readl(pmc->block[blk_num].mmio_base +
+ MLXBF_PMC_CRSPACE_PERFMON_CTL(pmc->block[blk_num].counters),
+ &reg))
+ return -EINVAL;
+
+ value = FIELD_GET(MLXBF_PMC_CRSPACE_PERFMON_UOC, reg);
+
+ return sysfs_emit(buf, "%u\n", value);
+}
+
+/* Store function for "use_odd_counter" sysfs files - only for crspace */
+static ssize_t mlxbf_pmc_use_odd_counter_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct mlxbf_pmc_attribute *attr_use_odd_counter = container_of(
+ attr, struct mlxbf_pmc_attribute, dev_attr);
+ unsigned int blk_num;
+ u32 uoc, reg;
+ int err;
+
+ blk_num = attr_use_odd_counter->nr;
+
+ err = kstrtouint(buf, 0, &uoc);
+ if (err < 0)
+ return err;
+
+ err = mlxbf_pmc_readl(pmc->block[blk_num].mmio_base +
+ MLXBF_PMC_CRSPACE_PERFMON_CTL(pmc->block[blk_num].counters),
+ &reg);
+ if (err)
+ return -EINVAL;
+
+ reg &= ~MLXBF_PMC_CRSPACE_PERFMON_UOC;
+ reg |= FIELD_PREP(MLXBF_PMC_CRSPACE_PERFMON_UOC, uoc);
+
+ mlxbf_pmc_write(pmc->block[blk_num].mmio_base +
+ MLXBF_PMC_CRSPACE_PERFMON_CTL(pmc->block[blk_num].counters),
+ MLXBF_PMC_WRITE_REG_32, reg);
+
+ return count;
+}
+
+/* Show function for "count_clock" sysfs files - only for crspace */
+static ssize_t mlxbf_pmc_count_clock_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct mlxbf_pmc_attribute *attr_count_clock = container_of(
+ attr, struct mlxbf_pmc_attribute, dev_attr);
+ unsigned int blk_num;
+ u32 reg;
+
+ blk_num = attr_count_clock->nr;
+
+ if (mlxbf_pmc_readl(pmc->block[blk_num].mmio_base +
+ MLXBF_PMC_CRSPACE_PERFMON_COUNT_CLOCK(pmc->block[blk_num].counters),
+ &reg))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%u\n", reg);
+}
+
+/* Store function for "count_clock" sysfs files - only for crspace */
+static ssize_t mlxbf_pmc_count_clock_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct mlxbf_pmc_attribute *attr_count_clock = container_of(
+ attr, struct mlxbf_pmc_attribute, dev_attr);
+ unsigned int blk_num;
+ u32 reg;
+ int err;
+
+ blk_num = attr_count_clock->nr;
+
+ err = kstrtouint(buf, 0, &reg);
+ if (err < 0)
+ return err;
+
+ mlxbf_pmc_write(pmc->block[blk_num].mmio_base +
+ MLXBF_PMC_CRSPACE_PERFMON_COUNT_CLOCK(pmc->block[blk_num].counters),
+ MLXBF_PMC_WRITE_REG_32, reg);
+
+ return count;
+}
+
/* Populate attributes for blocks with counters to monitor performance */
static int mlxbf_pmc_init_perftype_counter(struct device *dev, unsigned int blk_num)
{
@@ -1794,6 +1897,37 @@ static int mlxbf_pmc_init_perftype_counter(struct device *dev, unsigned int blk_
attr = NULL;
}

+ if (pmc->block[blk_num].type == MLXBF_PMC_TYPE_CRSPACE) {
+ /*
+ * Couple adjacent odd and even 32-bit counters to form 64-bit counters
+ * using "use_odd_counter" sysfs which has one bit per even counter.
+ */
+ attr = &pmc->block[blk_num].attr_use_odd_counter;
+ attr->dev_attr.attr.mode = 0644;
+ attr->dev_attr.show = mlxbf_pmc_use_odd_counter_show;
+ attr->dev_attr.store = mlxbf_pmc_use_odd_counter_store;
+ attr->nr = blk_num;
+ attr->dev_attr.attr.name = devm_kasprintf(dev, GFP_KERNEL,
+ "use_odd_counter");
+ if (!attr->dev_attr.attr.name)
+ return -ENOMEM;
+ pmc->block[blk_num].block_attr[++i] = &attr->dev_attr.attr;
+ attr = NULL;
+
+ /* Program crspace counters to count clock cycles using "count_clock" sysfs */
+ attr = &pmc->block[blk_num].attr_count_clock;
+ attr->dev_attr.attr.mode = 0644;
+ attr->dev_attr.show = mlxbf_pmc_count_clock_show;
+ attr->dev_attr.store = mlxbf_pmc_count_clock_store;
+ attr->nr = blk_num;
+ attr->dev_attr.attr.name = devm_kasprintf(dev, GFP_KERNEL,
+ "count_clock");
+ if (!attr->dev_attr.attr.name)
+ return -ENOMEM;
+ pmc->block[blk_num].block_attr[++i] = &attr->dev_attr.attr;
+ attr = NULL;
+ }
+
pmc->block[blk_num].attr_counter = devm_kcalloc(
dev, pmc->block[blk_num].counters,
sizeof(struct mlxbf_pmc_attribute), GFP_KERNEL);
--
2.30.1


2024-02-09 08:41:31

by Shravan Ramani

[permalink] [raw]
Subject: [PATCH v2 4/4] platform/mellanox: mlxbf-pmc: Add support for clock_measure performance block

The HW clock_measure counter info is passed to the driver from ACPI.
Create a new sub-directory for clock_measure events and provide
read access to the user. Writes are blocked since the fields are RO.

Signed-off-by: Shravan Kumar Ramani <[email protected]>
Reviewed-by: David Thompson <[email protected]>
Reviewed-by: Vadim Pasternak <[email protected]>
---
drivers/platform/mellanox/mlxbf-pmc.c | 46 ++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
index 1b9356a3ffab..565951c69a0f 100644
--- a/drivers/platform/mellanox/mlxbf-pmc.c
+++ b/drivers/platform/mellanox/mlxbf-pmc.c
@@ -865,6 +865,37 @@ static const struct mlxbf_pmc_events mlxbf_pmc_llt_miss_events[] = {
{75, "HISTOGRAM_HISTOGRAM_BIN9"},
};

+static const struct mlxbf_pmc_events mlxbf_pmc_clock_events[] = {
+ { 0x0, "FMON_CLK_LAST_COUNT_PLL_D1_INST0" },
+ { 0x4, "REFERENCE_WINDOW_WIDTH_PLL_D1_INST0" },
+ { 0x8, "FMON_CLK_LAST_COUNT_PLL_D1_INST1" },
+ { 0xc, "REFERENCE_WINDOW_WIDTH_PLL_D1_INST1" },
+ { 0x10, "FMON_CLK_LAST_COUNT_PLL_G1" },
+ { 0x14, "REFERENCE_WINDOW_WIDTH_PLL_G1" },
+ { 0x18, "FMON_CLK_LAST_COUNT_PLL_W1" },
+ { 0x1c, "REFERENCE_WINDOW_WIDTH_PLL_W1" },
+ { 0x20, "FMON_CLK_LAST_COUNT_PLL_T1" },
+ { 0x24, "REFERENCE_WINDOW_WIDTH_PLL_T1" },
+ { 0x28, "FMON_CLK_LAST_COUNT_PLL_A0" },
+ { 0x2c, "REFERENCE_WINDOW_WIDTH_PLL_A0" },
+ { 0x30, "FMON_CLK_LAST_COUNT_PLL_C0" },
+ { 0x34, "REFERENCE_WINDOW_WIDTH_PLL_C0" },
+ { 0x38, "FMON_CLK_LAST_COUNT_PLL_N1" },
+ { 0x3c, "REFERENCE_WINDOW_WIDTH_PLL_N1" },
+ { 0x40, "FMON_CLK_LAST_COUNT_PLL_I1" },
+ { 0x44, "REFERENCE_WINDOW_WIDTH_PLL_I1" },
+ { 0x48, "FMON_CLK_LAST_COUNT_PLL_R1" },
+ { 0x4c, "REFERENCE_WINDOW_WIDTH_PLL_R1" },
+ { 0x50, "FMON_CLK_LAST_COUNT_PLL_P1" },
+ { 0x54, "REFERENCE_WINDOW_WIDTH_PLL_P1" },
+ { 0x58, "FMON_CLK_LAST_COUNT_REF_100_INST0" },
+ { 0x5c, "REFERENCE_WINDOW_WIDTH_REF_100_INST0" },
+ { 0x60, "FMON_CLK_LAST_COUNT_REF_100_INST1" },
+ { 0x64, "REFERENCE_WINDOW_WIDTH_REF_100_INST1" },
+ { 0x68, "FMON_CLK_LAST_COUNT_REF_156" },
+ { 0x6c, "REFERENCE_WINDOW_WIDTH_REF_156" },
+};
+
static struct mlxbf_pmc_context *pmc;

/* UUID used to probe ATF service. */
@@ -1041,6 +1072,9 @@ static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk,
} else if (strstr(blk, "llt")) {
events = mlxbf_pmc_llt_events;
*size = ARRAY_SIZE(mlxbf_pmc_llt_events);
+ } else if (strstr(blk, "clock_measure")) {
+ events = mlxbf_pmc_clock_events;
+ *size = ARRAY_SIZE(mlxbf_pmc_clock_events);
} else {
events = NULL;
*size = 0;
@@ -1474,14 +1508,15 @@ static int mlxbf_pmc_read_event(unsigned int blk_num, u32 cnt_num, bool is_l3,
/* Method to read a register */
static int mlxbf_pmc_read_reg(unsigned int blk_num, u32 offset, u64 *result)
{
- u32 ecc_out;
+ u32 reg;

- if (strstr(pmc->block_name[blk_num], "ecc")) {
+ if ((strstr(pmc->block_name[blk_num], "ecc")) ||
+ (strstr(pmc->block_name[blk_num], "clock_measure"))) {
if (mlxbf_pmc_readl(pmc->block[blk_num].mmio_base + offset,
- &ecc_out))
+ &reg))
return -EFAULT;

- *result = ecc_out;
+ *result = reg;
return 0;
}

@@ -1495,6 +1530,9 @@ static int mlxbf_pmc_read_reg(unsigned int blk_num, u32 offset, u64 *result)
/* Method to write to a register */
static int mlxbf_pmc_write_reg(unsigned int blk_num, u32 offset, u64 data)
{
+ if (strstr(pmc->block_name[blk_num], "clock_measure"))
+ return -EINVAL;
+
if (strstr(pmc->block_name[blk_num], "ecc")) {
return mlxbf_pmc_write(pmc->block[blk_num].mmio_base + offset,
MLXBF_PMC_WRITE_REG_32, data);
--
2.30.1


2024-02-09 08:42:45

by Shravan Ramani

[permalink] [raw]
Subject: [PATCH v2 2/4] platform/mellanox: mlxbf-pmc: Fix signed/unsigned mix-up

Signed-off-by: Shravan Kumar Ramani <[email protected]>
---
drivers/platform/mellanox/mlxbf-pmc.c | 110 ++++++++++++++------------
1 file changed, 58 insertions(+), 52 deletions(-)

diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
index 71d919832e2a..e3f1ae772e43 100644
--- a/drivers/platform/mellanox/mlxbf-pmc.c
+++ b/drivers/platform/mellanox/mlxbf-pmc.c
@@ -99,8 +99,8 @@
*/
struct mlxbf_pmc_attribute {
struct device_attribute dev_attr;
- int index;
- int nr;
+ unsigned int index;
+ unsigned int nr;
};

/**
@@ -121,7 +121,7 @@ struct mlxbf_pmc_block_info {
void __iomem *mmio_base;
size_t blk_size;
size_t counters;
- int type;
+ unsigned int type;
struct mlxbf_pmc_attribute *attr_counter;
struct mlxbf_pmc_attribute *attr_event;
struct mlxbf_pmc_attribute attr_event_list;
@@ -169,7 +169,7 @@ struct mlxbf_pmc_context {
* @evt_name: Name of the event
*/
struct mlxbf_pmc_events {
- int evt_num;
+ u32 evt_num;
char *evt_name;
};

@@ -959,7 +959,7 @@ static int mlxbf_pmc_write(void __iomem *addr, int command, u64 value)
}

/* Check if the register offset is within the mapped region for the block */
-static bool mlxbf_pmc_valid_range(int blk_num, u32 offset)
+static bool mlxbf_pmc_valid_range(unsigned int blk_num, u32 offset)
{
if ((offset >= 0) && !(offset % MLXBF_PMC_REG_SIZE) &&
(offset + MLXBF_PMC_REG_SIZE <= pmc->block[blk_num].blk_size))
@@ -970,7 +970,7 @@ static bool mlxbf_pmc_valid_range(int blk_num, u32 offset)

/* Get the event list corresponding to a certain block */
static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk,
- int *size)
+ unsigned int *size)
{
const struct mlxbf_pmc_events *events;

@@ -1047,7 +1047,7 @@ static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk,
static int mlxbf_pmc_get_event_num(const char *blk, const char *evt)
{
const struct mlxbf_pmc_events *events;
- int i, size;
+ unsigned int size, i;

events = mlxbf_pmc_event_list(blk, &size);
if (!events)
@@ -1062,10 +1062,10 @@ static int mlxbf_pmc_get_event_num(const char *blk, const char *evt)
}

/* Get the event number given the name */
-static char *mlxbf_pmc_get_event_name(const char *blk, int evt)
+static char *mlxbf_pmc_get_event_name(const char *blk, u32 evt)
{
const struct mlxbf_pmc_events *events;
- int i, size;
+ unsigned int size, i;

events = mlxbf_pmc_event_list(blk, &size);
if (!events)
@@ -1080,7 +1080,7 @@ static char *mlxbf_pmc_get_event_name(const char *blk, int evt)
}

/* Method to enable/disable/reset l3cache counters */
-static int mlxbf_pmc_config_l3_counters(int blk_num, bool enable, bool reset)
+static int mlxbf_pmc_config_l3_counters(unsigned int blk_num, bool enable, bool reset)
{
u32 perfcnt_cfg = 0;

@@ -1095,7 +1095,7 @@ static int mlxbf_pmc_config_l3_counters(int blk_num, bool enable, bool reset)
}

/* Method to handle l3cache counter programming */
-static int mlxbf_pmc_program_l3_counter(int blk_num, u32 cnt_num,
+static int mlxbf_pmc_program_l3_counter(unsigned int blk_num, u32 cnt_num,
u32 evt)
{
u32 perfcnt_sel_1 = 0, perfcnt_sel = 0, *wordaddr;
@@ -1160,7 +1160,7 @@ static int mlxbf_pmc_program_l3_counter(int blk_num, u32 cnt_num,
}

/* Method to handle crspace counter programming */
-static int mlxbf_pmc_program_crspace_counter(int blk_num, u32 cnt_num,
+static int mlxbf_pmc_program_crspace_counter(unsigned int blk_num, u32 cnt_num,
u32 evt)
{
void *addr;
@@ -1185,7 +1185,7 @@ static int mlxbf_pmc_program_crspace_counter(int blk_num, u32 cnt_num,
}

/* Method to clear crspace counter value */
-static int mlxbf_pmc_clear_crspace_counter(int blk_num, u32 cnt_num)
+static int mlxbf_pmc_clear_crspace_counter(unsigned int blk_num, u32 cnt_num)
{
void *addr;

@@ -1197,7 +1197,7 @@ static int mlxbf_pmc_clear_crspace_counter(int blk_num, u32 cnt_num)
}

/* Method to program a counter to monitor an event */
-static int mlxbf_pmc_program_counter(int blk_num, u32 cnt_num,
+static int mlxbf_pmc_program_counter(unsigned int blk_num, u32 cnt_num,
u32 evt, bool is_l3)
{
u64 perfctl, perfevt, perfmon_cfg;
@@ -1261,7 +1261,7 @@ static int mlxbf_pmc_program_counter(int blk_num, u32 cnt_num,
}

/* Method to handle l3 counter reads */
-static int mlxbf_pmc_read_l3_counter(int blk_num, u32 cnt_num,
+static int mlxbf_pmc_read_l3_counter(unsigned int blk_num, u32 cnt_num,
u64 *result)
{
u32 perfcnt_low = 0, perfcnt_high = 0;
@@ -1293,7 +1293,7 @@ static int mlxbf_pmc_read_l3_counter(int blk_num, u32 cnt_num,
}

/* Method to handle crspace counter reads */
-static int mlxbf_pmc_read_crspace_counter(int blk_num, u32 cnt_num,
+static int mlxbf_pmc_read_crspace_counter(unsigned int blk_num, u32 cnt_num,
u64 *result)
{
int status = 0;
@@ -1311,7 +1311,7 @@ static int mlxbf_pmc_read_crspace_counter(int blk_num, u32 cnt_num,
}

/* Method to read the counter value */
-static int mlxbf_pmc_read_counter(int blk_num, u32 cnt_num, bool is_l3,
+static int mlxbf_pmc_read_counter(unsigned int blk_num, u32 cnt_num, bool is_l3,
u64 *result)
{
u32 perfcfg_offset, perfval_offset;
@@ -1349,7 +1349,7 @@ static int mlxbf_pmc_read_counter(int blk_num, u32 cnt_num, bool is_l3,
}

/* Method to read L3 block event */
-static int mlxbf_pmc_read_l3_event(int blk_num, u32 cnt_num,
+static int mlxbf_pmc_read_l3_event(unsigned int blk_num, u32 cnt_num,
u64 *result)
{
u32 perfcnt_sel = 0, perfcnt_sel_1 = 0, *wordaddr;
@@ -1402,7 +1402,7 @@ static int mlxbf_pmc_read_l3_event(int blk_num, u32 cnt_num,
}

/* Method to read crspace block event */
-static int mlxbf_pmc_read_crspace_event(int blk_num, u32 cnt_num,
+static int mlxbf_pmc_read_crspace_event(unsigned int blk_num, u32 cnt_num,
u64 *result)
{
u32 word, evt;
@@ -1426,7 +1426,7 @@ static int mlxbf_pmc_read_crspace_event(int blk_num, u32 cnt_num,
}

/* Method to find the event currently being monitored by a counter */
-static int mlxbf_pmc_read_event(int blk_num, u32 cnt_num, bool is_l3,
+static int mlxbf_pmc_read_event(unsigned int blk_num, u32 cnt_num, bool is_l3,
u64 *result)
{
u32 perfcfg_offset, perfval_offset;
@@ -1466,7 +1466,7 @@ static int mlxbf_pmc_read_event(int blk_num, u32 cnt_num, bool is_l3,
}

/* Method to read a register */
-static int mlxbf_pmc_read_reg(int blk_num, u32 offset, u64 *result)
+static int mlxbf_pmc_read_reg(unsigned int blk_num, u32 offset, u64 *result)
{
u32 ecc_out;

@@ -1487,7 +1487,7 @@ static int mlxbf_pmc_read_reg(int blk_num, u32 offset, u64 *result)
}

/* Method to write to a register */
-static int mlxbf_pmc_write_reg(int blk_num, u32 offset, u64 data)
+static int mlxbf_pmc_write_reg(unsigned int blk_num, u32 offset, u64 data)
{
if (strstr(pmc->block_name[blk_num], "ecc")) {
return mlxbf_pmc_write(pmc->block[blk_num].mmio_base + offset,
@@ -1507,7 +1507,7 @@ static ssize_t mlxbf_pmc_counter_show(struct device *dev,
{
struct mlxbf_pmc_attribute *attr_counter = container_of(
attr, struct mlxbf_pmc_attribute, dev_attr);
- int blk_num, cnt_num, offset;
+ unsigned int blk_num, cnt_num, offset;
bool is_l3 = false;
u64 value;

@@ -1541,14 +1541,15 @@ static ssize_t mlxbf_pmc_counter_store(struct device *dev,
{
struct mlxbf_pmc_attribute *attr_counter = container_of(
attr, struct mlxbf_pmc_attribute, dev_attr);
- int blk_num, cnt_num, offset, err, data;
+ unsigned int blk_num, cnt_num, offset, data;
bool is_l3 = false;
u64 evt_num;
+ int err;

blk_num = attr_counter->nr;
cnt_num = attr_counter->index;

- err = kstrtoint(buf, 0, &data);
+ err = kstrtouint(buf, 0, &data);
if (err < 0)
return err;

@@ -1577,7 +1578,7 @@ static ssize_t mlxbf_pmc_counter_store(struct device *dev,
if (err)
return err;
} else if (pmc->block[blk_num].type == MLXBF_PMC_TYPE_CRSPACE) {
- if (sscanf(attr->attr.name, "counter%d", &cnt_num) != 1)
+ if (sscanf(attr->attr.name, "counter%u", &cnt_num) != 1)
return -EINVAL;
err = mlxbf_pmc_clear_crspace_counter(blk_num, cnt_num);
} else
@@ -1592,10 +1593,11 @@ static ssize_t mlxbf_pmc_event_show(struct device *dev,
{
struct mlxbf_pmc_attribute *attr_event = container_of(
attr, struct mlxbf_pmc_attribute, dev_attr);
- int blk_num, cnt_num, err;
+ unsigned int blk_num, cnt_num;
bool is_l3 = false;
- u64 evt_num;
char *evt_name;
+ u64 evt_num;
+ int err;

blk_num = attr_event->nr;
cnt_num = attr_event->index;
@@ -1621,8 +1623,9 @@ static ssize_t mlxbf_pmc_event_store(struct device *dev,
{
struct mlxbf_pmc_attribute *attr_event = container_of(
attr, struct mlxbf_pmc_attribute, dev_attr);
- int blk_num, cnt_num, evt_num, err;
+ unsigned int blk_num, cnt_num, evt_num;
bool is_l3 = false;
+ int err;

blk_num = attr_event->nr;
cnt_num = attr_event->index;
@@ -1633,7 +1636,7 @@ static ssize_t mlxbf_pmc_event_store(struct device *dev,
if (evt_num < 0)
return -EINVAL;
} else {
- err = kstrtoint(buf, 0, &evt_num);
+ err = kstrtouint(buf, 0, &evt_num);
if (err < 0)
return err;
}
@@ -1655,9 +1658,10 @@ static ssize_t mlxbf_pmc_event_list_show(struct device *dev,
{
struct mlxbf_pmc_attribute *attr_event_list = container_of(
attr, struct mlxbf_pmc_attribute, dev_attr);
- int blk_num, i, size, len = 0, ret = 0;
+ unsigned int blk_num, i, size, len = 0;
const struct mlxbf_pmc_events *events;
char e_info[MLXBF_PMC_EVENT_INFO_LEN];
+ int ret = 0;

blk_num = attr_event_list->nr;

@@ -1683,8 +1687,8 @@ static ssize_t mlxbf_pmc_enable_show(struct device *dev,
{
struct mlxbf_pmc_attribute *attr_enable = container_of(
attr, struct mlxbf_pmc_attribute, dev_attr);
+ unsigned int blk_num, value;
u32 perfcnt_cfg, word;
- int blk_num, value;

blk_num = attr_enable->nr;

@@ -1704,7 +1708,7 @@ static ssize_t mlxbf_pmc_enable_show(struct device *dev,
value = FIELD_GET(MLXBF_PMC_L3C_PERF_CNT_CFG_EN, perfcnt_cfg);
}

- return sysfs_emit(buf, "%d\n", value);
+ return sysfs_emit(buf, "%u\n", value);
}

/* Store function for "enable" sysfs files - only for l3cache & crspace */
@@ -1714,12 +1718,13 @@ static ssize_t mlxbf_pmc_enable_store(struct device *dev,
{
struct mlxbf_pmc_attribute *attr_enable = container_of(
attr, struct mlxbf_pmc_attribute, dev_attr);
- int err, en, blk_num;
+ unsigned int en, blk_num;
u32 word;
+ int err;

blk_num = attr_enable->nr;

- err = kstrtoint(buf, 0, &en);
+ err = kstrtouint(buf, 0, &en);
if (err < 0)
return err;

@@ -1757,10 +1762,10 @@ static ssize_t mlxbf_pmc_enable_store(struct device *dev,
}

/* Populate attributes for blocks with counters to monitor performance */
-static int mlxbf_pmc_init_perftype_counter(struct device *dev, int blk_num)
+static int mlxbf_pmc_init_perftype_counter(struct device *dev, unsigned int blk_num)
{
struct mlxbf_pmc_attribute *attr;
- int i = 0, j = 0;
+ unsigned int i = 0, j = 0;

/* "event_list" sysfs to list events supported by the block */
attr = &pmc->block[blk_num].attr_event_list;
@@ -1810,7 +1815,7 @@ static int mlxbf_pmc_init_perftype_counter(struct device *dev, int blk_num)
attr->index = j;
attr->nr = blk_num;
attr->dev_attr.attr.name = devm_kasprintf(dev, GFP_KERNEL,
- "counter%d", j);
+ "counter%u", j);
if (!attr->dev_attr.attr.name)
return -ENOMEM;
pmc->block[blk_num].block_attr[++i] = &attr->dev_attr.attr;
@@ -1823,7 +1828,7 @@ static int mlxbf_pmc_init_perftype_counter(struct device *dev, int blk_num)
attr->index = j;
attr->nr = blk_num;
attr->dev_attr.attr.name = devm_kasprintf(dev, GFP_KERNEL,
- "event%d", j);
+ "event%u", j);
if (!attr->dev_attr.attr.name)
return -ENOMEM;
pmc->block[blk_num].block_attr[++i] = &attr->dev_attr.attr;
@@ -1834,11 +1839,11 @@ static int mlxbf_pmc_init_perftype_counter(struct device *dev, int blk_num)
}

/* Populate attributes for blocks with registers to monitor performance */
-static int mlxbf_pmc_init_perftype_reg(struct device *dev, int blk_num)
+static int mlxbf_pmc_init_perftype_reg(struct device *dev, unsigned int blk_num)
{
- struct mlxbf_pmc_attribute *attr;
const struct mlxbf_pmc_events *events;
- int i = 0, j = 0;
+ struct mlxbf_pmc_attribute *attr;
+ unsigned int i = 0, j = 0;

events = mlxbf_pmc_event_list(pmc->block_name[blk_num], &j);
if (!events)
@@ -1869,7 +1874,7 @@ static int mlxbf_pmc_init_perftype_reg(struct device *dev, int blk_num)
}

/* Helper to create the bfperf sysfs sub-directories and files */
-static int mlxbf_pmc_create_groups(struct device *dev, int blk_num)
+static int mlxbf_pmc_create_groups(struct device *dev, unsigned int blk_num)
{
int err;

@@ -1912,18 +1917,19 @@ static bool mlxbf_pmc_guid_match(const guid_t *guid,
static int mlxbf_pmc_map_counters(struct device *dev)
{
u64 info[MLXBF_PMC_INFO_SZ];
- int i, tile_num, ret;
+ unsigned int tile_num, i;
+ int ret;

for (i = 0; i < pmc->total_blocks; ++i) {
/* Create sysfs for tiles only if block number < tile_count */
if (strstr(pmc->block_name[i], "tilenet")) {
- if (sscanf(pmc->block_name[i], "tilenet%d", &tile_num) != 1)
+ if (sscanf(pmc->block_name[i], "tilenet%u", &tile_num) != 1)
continue;

if (tile_num >= pmc->tile_count)
continue;
} else if (strstr(pmc->block_name[i], "tile")) {
- if (sscanf(pmc->block_name[i], "tile%d", &tile_num) != 1)
+ if (sscanf(pmc->block_name[i], "tile%u", &tile_num) != 1)
continue;

if (tile_num >= pmc->tile_count)
@@ -1933,9 +1939,9 @@ static int mlxbf_pmc_map_counters(struct device *dev)
/* Create sysfs only for enabled MSS blocks */
if (strstr(pmc->block_name[i], "mss") &&
pmc->event_set == MLXBF_PMC_EVENT_SET_BF3) {
- int mss_num;
+ unsigned int mss_num;

- if (sscanf(pmc->block_name[i], "mss%d", &mss_num) != 1)
+ if (sscanf(pmc->block_name[i], "mss%u", &mss_num) != 1)
continue;

if (!((pmc->mss_enable >> mss_num) & 0x1))
@@ -1944,17 +1950,17 @@ static int mlxbf_pmc_map_counters(struct device *dev)

/* Create sysfs only for enabled LLT blocks */
if (strstr(pmc->block_name[i], "llt_miss")) {
- int llt_num;
+ unsigned int llt_num;

- if (sscanf(pmc->block_name[i], "llt_miss%d", &llt_num) != 1)
+ if (sscanf(pmc->block_name[i], "llt_miss%u", &llt_num) != 1)
continue;

if (!((pmc->llt_enable >> llt_num) & 0x1))
continue;
} else if (strstr(pmc->block_name[i], "llt")) {
- int llt_num;
+ unsigned int llt_num;

- if (sscanf(pmc->block_name[i], "llt%d", &llt_num) != 1)
+ if (sscanf(pmc->block_name[i], "llt%u", &llt_num) != 1)
continue;

if (!((pmc->llt_enable >> llt_num) & 0x1))
--
2.30.1


2024-02-09 15:08:37

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] platform/mellanox: mlxbf-pmc: Fix signed/unsigned mix-up

On Fri, 9 Feb 2024, Shravan Kumar Ramani wrote:

> Signed-off-by: Shravan Kumar Ramani <[email protected]>
> ---
> drivers/platform/mellanox/mlxbf-pmc.c | 110 ++++++++++++++------------
> 1 file changed, 58 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
> index 71d919832e2a..e3f1ae772e43 100644
> --- a/drivers/platform/mellanox/mlxbf-pmc.c
> +++ b/drivers/platform/mellanox/mlxbf-pmc.c
> @@ -99,8 +99,8 @@
> */
> struct mlxbf_pmc_attribute {
> struct device_attribute dev_attr;
> - int index;
> - int nr;
> + unsigned int index;
> + unsigned int nr;
> };
>
> /**
> @@ -121,7 +121,7 @@ struct mlxbf_pmc_block_info {
> void __iomem *mmio_base;
> size_t blk_size;
> size_t counters;
> - int type;
> + unsigned int type;
> struct mlxbf_pmc_attribute *attr_counter;
> struct mlxbf_pmc_attribute *attr_event;
> struct mlxbf_pmc_attribute attr_event_list;
> @@ -169,7 +169,7 @@ struct mlxbf_pmc_context {
> * @evt_name: Name of the event
> */
> struct mlxbf_pmc_events {
> - int evt_num;
> + u32 evt_num;
> char *evt_name;
> };
>
> @@ -959,7 +959,7 @@ static int mlxbf_pmc_write(void __iomem *addr, int command, u64 value)
> }
>
> /* Check if the register offset is within the mapped region for the block */
> -static bool mlxbf_pmc_valid_range(int blk_num, u32 offset)
> +static bool mlxbf_pmc_valid_range(unsigned int blk_num, u32 offset)
> {
> if ((offset >= 0) && !(offset % MLXBF_PMC_REG_SIZE) &&
> (offset + MLXBF_PMC_REG_SIZE <= pmc->block[blk_num].blk_size))
> @@ -970,7 +970,7 @@ static bool mlxbf_pmc_valid_range(int blk_num, u32 offset)
>
> /* Get the event list corresponding to a certain block */
> static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk,
> - int *size)
> + unsigned int *size)

Usually size_t is the type that is preferred for sizes of memory blocks
(including this case where it's array size that is just memory block
size divided by another).

--
i.


2024-02-09 19:53:54

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] platform/mellanox: mlxbf-pmc: Replace uintN_t with kernel-style types

On Fri, 9 Feb 2024, Shravan Kumar Ramani wrote:

Hi,

You need to provice commit description here too. The shortlog on subject
line is not enough.

> Signed-off-by: Shravan Kumar Ramani <[email protected]>
> ---
> drivers/platform/mellanox/mlxbf-pmc.c | 121 +++++++++++++-------------
> 1 file changed, 59 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
> index b1995ac268d7..71d919832e2a 100644
> --- a/drivers/platform/mellanox/mlxbf-pmc.c
> +++ b/drivers/platform/mellanox/mlxbf-pmc.c
> @@ -149,17 +149,17 @@ struct mlxbf_pmc_block_info {
> */
> struct mlxbf_pmc_context {
> struct platform_device *pdev;
> - uint32_t total_blocks;
> - uint32_t tile_count;
> - uint8_t llt_enable;
> - uint8_t mss_enable;
> - uint32_t group_num;
> + u32 total_blocks;
> + u32 tile_count;
> + u8 llt_enable;
> + u8 mss_enable;
> + u32 group_num;
> struct device *hwmon_dev;
> const char *block_name[MLXBF_PMC_MAX_BLOCKS];
> struct mlxbf_pmc_block_info block[MLXBF_PMC_MAX_BLOCKS];
> const struct attribute_group *groups[MLXBF_PMC_MAX_BLOCKS];
> bool svc_sreg_support;
> - uint32_t sreg_tbl_perf;
> + u32 sreg_tbl_perf;
> unsigned int event_set;
> };
>
> @@ -865,8 +865,8 @@ static struct mlxbf_pmc_context *pmc;
> static const char *mlxbf_pmc_svc_uuid_str = "89c036b4-e7d7-11e6-8797-001aca00bfc4";
>
> /* Calls an SMC to access a performance register */
> -static int mlxbf_pmc_secure_read(void __iomem *addr, uint32_t command,
> - uint64_t *result)
> +static int mlxbf_pmc_secure_read(void __iomem *addr, u32 command,
> + u64 *result)

Please remove unnecessary newlines too such as this one from the function
arguments.

Other than those two things, this one looked fine. Thanks for doing this.


--
i.