> -----Original Message----- > From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > Sent: Monday, July 27, 2020 4:24 PM > To: Shravan Ramani <sramani@xxxxxxxxxxxx> > Cc: Andy Shevchenko <andy@xxxxxxxxxxxxx>; Darren Hart > <dvhart@xxxxxxxxxxxxx>; Vadim Pasternak <vadimp@xxxxxxxxxxxx>; Jiri Pirko > <jiri@xxxxxxxxxxxx>; Platform Driver <platform-driver-x86@xxxxxxxxxxxxxxx>; > Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH v1] platform/mellanox: mlxbf-pmc: Add Mellanox BlueField > PMC driver > > On Mon, Jul 27, 2020 at 12:02 PM Shravan Kumar Ramani > <sramani@xxxxxxxxxxxx> wrote: > > > > The performance modules in BlueField are present in several hardware > > blocks and each block provides access to these stats either through > > counters that can be programmed to monitor supported events or through > > memory-mapped registers that hold the relevant information. > > The hardware blocks that include a performance module are: > > * Tile (block containing 2 cores and a shared L2 cache) > > * TRIO (PCIe root complex) > > * MSS (Memory Sub-system containing the Memory Controller and L3 > > cache) > > * GIC (Interrupt controller) > > * SMMU (System Memory Management Unit) The mlx_pmc driver provides > > access to all of these performance modules through a hwmon sysfs > > interface. > > Just brief comments: > - consider to revisit header block to see what is really necessary and what can > be dropped > - add comma to the arrays where last line is not a termination > - look at match_string() / sysfs_match_string() API, I think they can be utilised > here > - UUID manipulations (esp. with that GUID_INIT() against non-constant) seems > too much, consider refactoring and cleaning up these pieces Could you please elaborate on what approach you'd like me to take with the UUID manipulation? I used the same approach as in drivers/platform/mellanox/mlxbf-bootctl.c which seemed like an appropriate example. Any other pointers would be helpful. Thanks for the feedback. Will address all the other comments in v2. Regards, Shravan > - use kstroto*() API instead of sscanf. It has a range check > > > -- > With Best Regards, > Andy Shevchenko
