All of lore.kernel.org
 help / color / mirror / Atom feed
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


  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.