From: Suzuki.Poulose@arm.com (Suzuki K. Poulose)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 3/4] arm-cci: Add routines to enable/disable all counters
Date: Thu, 5 Nov 2015 10:19:50 +0000 [thread overview]
Message-ID: <563B2D46.10503@arm.com> (raw)
In-Reply-To: <563B2C01.80701@arm.com>
On 05/11/15 10:14, Suzuki K. Poulose wrote:
> On 04/11/15 18:28, Mark Rutland wrote:
>> On Tue, Oct 20, 2015 at 02:05:25PM +0100, Suzuki K. Poulose wrote:
>>> Adds helper routines to manipulate the counter controls for
>>> all the counters on the CCI PMU.
>>> +static void pmu_disable_counters_ctrl(struct cci_pmu *cci_pmu, unsigned long *mask)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < cci_pmu->num_cntrs; i++) {
>>> + clear_bit(i, mask);
>>> + if (pmu_get_counter_ctrl(cci_pmu, i)) {
>>> + set_bit(i, mask);
>>> + pmu_disable_counter(cci_pmu, i);
>>> + }
>>> + }
>>> +}
>>
>> I don't understand what's going on with the mask here. Why do we clear
>> ieach bit when the only user (introduced in the next patch) explicitly
>> clears the mask anyway?
>
> To be more precise, it should have been :
>
> if (pmu_get_counter_ctrl(cci_pmu, i)) {
> set_bit(i, mask);
> pmu_disable_counter(cci_pmu, i);
> } else
> clear_bit(i, mask);
Forgot to mention, the explicit clearing is for the bits that may be
beyond the num_counters. Since we limit it to cci_pmu->num_cntrs here
we could get rid of that.
Thanks
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: [PATCHv2 3/4] arm-cci: Add routines to enable/disable all counters
Date: Thu, 5 Nov 2015 10:19:50 +0000 [thread overview]
Message-ID: <563B2D46.10503@arm.com> (raw)
In-Reply-To: <563B2C01.80701@arm.com>
On 05/11/15 10:14, Suzuki K. Poulose wrote:
> On 04/11/15 18:28, Mark Rutland wrote:
>> On Tue, Oct 20, 2015 at 02:05:25PM +0100, Suzuki K. Poulose wrote:
>>> Adds helper routines to manipulate the counter controls for
>>> all the counters on the CCI PMU.
>>> +static void pmu_disable_counters_ctrl(struct cci_pmu *cci_pmu, unsigned long *mask)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < cci_pmu->num_cntrs; i++) {
>>> + clear_bit(i, mask);
>>> + if (pmu_get_counter_ctrl(cci_pmu, i)) {
>>> + set_bit(i, mask);
>>> + pmu_disable_counter(cci_pmu, i);
>>> + }
>>> + }
>>> +}
>>
>> I don't understand what's going on with the mask here. Why do we clear
>> ieach bit when the only user (introduced in the next patch) explicitly
>> clears the mask anyway?
>
> To be more precise, it should have been :
>
> if (pmu_get_counter_ctrl(cci_pmu, i)) {
> set_bit(i, mask);
> pmu_disable_counter(cci_pmu, i);
> } else
> clear_bit(i, mask);
Forgot to mention, the explicit clearing is for the bits that may be
beyond the num_counters. Since we limit it to cci_pmu->num_cntrs here
we could get rid of that.
Thanks
Suzuki
next prev parent reply other threads:[~2015-11-05 10:19 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-20 13:05 [PATCHv2 0/4] arm-cci500: Workaround pmu_event_set_period Suzuki K. Poulose
2015-10-20 13:05 ` Suzuki K. Poulose
2015-10-20 13:05 ` [PATCHv2 1/4] arm-cci: Refactor CCI PMU code Suzuki K. Poulose
2015-10-20 13:05 ` Suzuki K. Poulose
2015-11-04 18:01 ` Mark Rutland
2015-11-04 18:01 ` Mark Rutland
2015-11-04 18:17 ` Suzuki K. Poulose
2015-11-04 18:17 ` Suzuki K. Poulose
2015-10-20 13:05 ` [PATCHv2 2/4] arm-cci: Get the status of a counter Suzuki K. Poulose
2015-10-20 13:05 ` Suzuki K. Poulose
2015-11-04 18:06 ` Mark Rutland
2015-11-04 18:06 ` Mark Rutland
2015-11-04 18:20 ` Suzuki K. Poulose
2015-11-04 18:20 ` Suzuki K. Poulose
2015-10-20 13:05 ` [PATCHv2 3/4] arm-cci: Add routines to enable/disable all counters Suzuki K. Poulose
2015-10-20 13:05 ` Suzuki K. Poulose
2015-11-04 18:28 ` Mark Rutland
2015-11-04 18:28 ` Mark Rutland
2015-11-05 10:14 ` Suzuki K. Poulose
2015-11-05 10:14 ` Suzuki K. Poulose
2015-11-05 10:19 ` Suzuki K. Poulose [this message]
2015-11-05 10:19 ` Suzuki K. Poulose
2015-11-05 17:27 ` Mark Rutland
2015-11-05 17:27 ` Mark Rutland
2015-11-05 17:52 ` Suzuki K. Poulose
2015-11-05 17:52 ` Suzuki K. Poulose
2015-10-20 13:05 ` [PATCHv2 4/4] arm-cci500: Work around PMU counter writes Suzuki K. Poulose
2015-10-20 13:05 ` Suzuki K. Poulose
2015-10-22 17:00 ` [PATCHv2 0/4] arm-cci500: Workaround pmu_event_set_period Olof Johansson
2015-10-22 17:00 ` Olof Johansson
2015-10-22 21:46 ` Suzuki K. Poulose
2015-10-22 21:46 ` Suzuki K. Poulose
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=563B2D46.10503@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.