lkml.org 
[lkml]   [2020]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v1] platform/mellanox: mlxbf-pmc: Add Mellanox BlueField PMC driver
Date


> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Sent: Monday, July 27, 2020 4:24 PM
> To: Shravan Ramani <sramani@mellanox.com>
> Cc: Andy Shevchenko <andy@infradead.org>; Darren Hart
> <dvhart@infradead.org>; Vadim Pasternak <vadimp@mellanox.com>; Jiri Pirko
> <jiri@mellanox.com>; Platform Driver <platform-driver-x86@vger.kernel.org>;
> Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
> 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@mellanox.com> 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
\
 
 \ /
  Last update: 2020-07-28 07:05    [W:2.158 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site