On Wed, Aug 21, 2019 at 10:23 PM Will Deacon <will@xxxxxxxxxx> wrote: > > On Tue, Aug 13, 2019 at 12:03:45PM +0100, Mark Rutland wrote: > > On Tue, Aug 13, 2019 at 04:25:15PM +0530, Ganapatrao Kulkarni wrote: > > > On Mon, Aug 12, 2019 at 5:31 PM Mark Rutland <mark.rutland@xxxxxxx> wrote: > > > > > > > > On Tue, Jul 23, 2019 at 09:16:28AM +0000, Ganapatrao Kulkarni wrote: > > > > > CCPI2 is a low-latency high-bandwidth serial interface for connecting > > > > > ThunderX2 processors. This patch adds support to capture CCPI2 perf events. > > > > > > > > It would be worth pointing out in the commit message how the CCPI2 > > > > counters differ from the others. I realise you have that in the body of > > > > patch 1, but it's critical information when reviewing this patch... > > > > > > Ok, I will add in next version. > > > > > > > > > > > > > > Signed-off-by: Ganapatrao Kulkarni <gkulkarni@xxxxxxxxxxx> > > > > > --- > > > > > drivers/perf/thunderx2_pmu.c | 248 ++++++++++++++++++++++++++++++----- > > > > > 1 file changed, 214 insertions(+), 34 deletions(-) > > > > > > > > > > diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c > > > > > index 43d76c85da56..a4e1273eafa3 100644 > > > > > --- a/drivers/perf/thunderx2_pmu.c > > > > > +++ b/drivers/perf/thunderx2_pmu.c > > > > > @@ -17,22 +17,31 @@ > > > > > */ > > > > > > > > > > #define TX2_PMU_MAX_COUNTERS 4 > > > > > > > > Shouldn't this be 8 now? > > > > > > It is kept unchanged to 4(as suggested by Will), which is same for > > > both L3 and DMC. > > > For CCPI2 this macro is not used. > > > > Hmmm.... > > > > I disagree with that suggestion given that this also affects the > > active_counters bitmap size (and thus it is not correctly sized as of > > this patch), and it doesn't really save us much. > > > > I think it would be better to bump this to 8 and always update the > > events array, even though it will be unused for CCPI2. That's less > > surprising, needs fewer special-cases, and we can use the hrtimer > > function pointer alone to determine if we need to do any hrtimer work. > > tbf, my complaint was actually about some macros applying to the whole > PMU whilst others refer only to DMC/L3C and this not being apparent from > the naming: > > https://lkml.org/lkml/2019/6/27/250 > > so I'm fine having TX2_PMU_DMC_L3C_MAX_COUNTERS and > TX2_PMU_CCPI2_MAX_COUNTERS, but that sort of naming needs to be consistent > unless the macro/definition really applies to both. That fed the suggestion > that GET_EVENTID could be generic and switch on the event type internally > instead of at the caller. Thanks Will for the clarification. I will send new version with changes as suggested. > > Will Thanks, Ganapat
