> On Wed, 17 Apr 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 > > 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@xxxxxxxxxx> > > Reviewed-by: David Thompson <davthompson@xxxxxxxxxx> > > Reviewed-by: Vadim Pasternak <vadimp@xxxxxxxxxx> > > --- > > > @@ -1799,6 +1902,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; > > + } > > Hi, > > What was the reason why this driver could not use .dev_groups to setup > sysfs (filtering can be done with .is_visible)? > The current approach was suggested during the initial submission of the driver and the same has been followed since. Do you mean to add a is_visible routine for each of the sysfs types like count_clock, use_odd_counter, etc and check the conditions for their inclusion in this routine? Thanks, Shravan