From: Suzuki.Poulose@arm.com (Suzuki K. Poulose)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv3 3/5] arm-cci: Add routines to enable/disable all counters
Date: Thu, 10 Dec 2015 15:42:41 +0000 [thread overview]
Message-ID: <56699D71.3070006@arm.com> (raw)
In-Reply-To: <20151210153222.GE495@leverpostej>
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
WARNING: multiple messages have this Message-ID (diff)
From: "Suzuki K. Poulose" <Suzuki.Poulose@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, punit.agrawal@arm.com,
arm@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv3 3/5] arm-cci: Add routines to enable/disable all counters
Date: Thu, 10 Dec 2015 15:42:41 +0000 [thread overview]
Message-ID: <56699D71.3070006@arm.com> (raw)
In-Reply-To: <20151210153222.GE495@leverpostej>
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
next prev parent reply other threads:[~2015-12-10 15:42 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-17 18:03 [PATCHv3 0/5] arm-cci500: Workaround pmu_event_set_period Suzuki K. Poulose
2015-11-17 18:03 ` Suzuki K. Poulose
2015-11-17 18:03 ` [PATCHv3 1/5] arm-cci: Refactor CCI PMU enable/disable methods Suzuki K. Poulose
2015-11-17 18:03 ` Suzuki K. Poulose
2015-12-10 15:26 ` Mark Rutland
2015-12-10 15:26 ` Mark Rutland
2015-11-17 18:03 ` [PATCHv3 2/5] arm-cci: Get the status of a counter Suzuki K. Poulose
2015-11-17 18:03 ` Suzuki K. Poulose
2015-12-10 15:33 ` Mark Rutland
2015-12-10 15:33 ` Mark Rutland
2015-11-17 18:03 ` [PATCHv3 3/5] arm-cci: Add routines to enable/disable all counters Suzuki K. Poulose
2015-11-17 18:03 ` Suzuki K. Poulose
2015-12-10 15:32 ` Mark Rutland
2015-12-10 15:32 ` Mark Rutland
2015-12-10 15:42 ` Suzuki K. Poulose [this message]
2015-12-10 15:42 ` Suzuki K. Poulose
2015-12-10 15:47 ` Mark Rutland
2015-12-10 15:47 ` Mark Rutland
2015-11-17 18:03 ` [PATCHv3 4/5] arm-cci: Add hooks for pmu_write_counter Suzuki K. Poulose
2015-11-17 18:03 ` Suzuki K. Poulose
2015-11-17 18:03 ` [PATCHv3 5/5] arm-cci: CCI-500: Work around PMU counter writes Suzuki K. Poulose
2015-11-17 18:03 ` Suzuki K. Poulose
2015-12-10 15:42 ` Mark Rutland
2015-12-10 15:42 ` Mark Rutland
2015-12-11 11:28 ` Suzuki K. Poulose
2015-12-11 11:28 ` Suzuki K. Poulose
2015-12-11 12:10 ` Peter Zijlstra
2015-12-11 12:10 ` Peter Zijlstra
2015-12-11 12:14 ` Mark Rutland
2015-12-11 12:14 ` Mark Rutland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56699D71.3070006@arm.com \
--to=suzuki.poulose@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.