Message ID | ce1140840b09df3fe2b85e3f87089f2e0d0877d0.1707808180.git.shravankr@nvidia.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | Updates to mlxbf-pmc | expand |
Hi, I've applied to first two type cleanup patches to review-ilpo. I considered taking patch 3 and 4 already too but I'd like to see the sysfs documentation for the added sysfs files first. On Tue, 13 Feb 2024, Shravan Kumar Ramani wrote: > Add support for programming any counter to monitor the cycle count. > Since counting of cycles using 32-bit ocunters would result in frequent ocunters -> counters > 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 Please try to avoid paragraphs like this. Either make it a truly flowing paragraph or insert a newline in between the paragraphs. > 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 <shravankr@nvidia.com> > Reviewed-by: David Thompson <davthompson@nvidia.com> > Reviewed-by: Vadim Pasternak <vadimp@nvidia.com>
Hi Shravan, On 2/13/24 12:15 PM, Shravan Kumar Ramani wrote: > 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 <shravankr@nvidia.com> > Reviewed-by: David Thompson <davthompson@nvidia.com> > Reviewed-by: Vadim Pasternak <vadimp@nvidia.com> As Ilpo already mentioned when adding new sysfs attributes these really should be documented, see: Documentation/ABI/testing/sysfs-platform-mellanox-bootctl for an example. It seems that the attributes used by mlxbf-pmc.c are not documented at all yet. Please start a new documentation file, e.g.: Documentation/ABI/testing/sysfs-platform-mellanox-pmc for this and at a minimum document the new sysfs attributes there. Ideally start with a seperate preparation patch documenting the existing attributes and then in v2 of this patch extend the docs with info on the new sysfs attributes, Regards, Hans > --- > 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 250405bb59a7..e2f11c0c63e9 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; > }; > @@ -1751,6 +1757,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), > + ®)) > + 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), > + ®); > + 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), > + ®)) > + 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, ®); > + 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) > { > @@ -1784,6 +1887,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);
diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c index 250405bb59a7..e2f11c0c63e9 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; }; @@ -1751,6 +1757,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), + ®)) + 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), + ®); + 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), + ®)) + 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, ®); + 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) { @@ -1784,6 +1887,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);