Re: [PATCHv3 3/5] arm-cci: Add routines to enable/disable all counters

From: Suzuki K. Poulose
Date: Thu Dec 10 2015 - 10:42:49 EST


On 10/12/15 15:32, Mark Rutland wrote:
On Tue, Nov 17, 2015 at 06:03:25PM +0000, Suzuki K. Poulose wrote:


+static void __maybe_unused
+pmu_disable_counters(struct cci_pmu *cci_pmu, unsigned long *mask)
+{
+ int i;
+
+ for (i = 0; i < cci_pmu->num_cntrs; i++) {
+ if (pmu_counter_is_enabled(cci_pmu, i)) {
+ set_bit(i, mask);
+ pmu_disable_counter(cci_pmu, i);
+ } else
+ clear_bit(i, mask);

Can we not assume a clean mask to begin with?

If we force the caller to pass a clean mask, yes we could. I am fine
with either approach.


+ }
+}
+
+/*
+ * Restore the status of the counters. Reversal of the pmu_disable_counters().
+ * For each counter set in the mask, enable the counter back.
+ */
+static void __maybe_unused
+pmu_restore_counters(struct cci_pmu *cci_pmu, unsigned long *mask)

This would probably be better with s/restore/enable/ for consistency
with pmu_disable_counters.

I had thought as well, but then chose restore as we don't enable all the
counters. Given that we pass a mask argument, it is fine to change it to
enable and will do that in the next one.


Other than that this looks fine to me.

Thanks for the review.

Cheers
Suzuki

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/