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

[Suzuki K. Poulose]

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

--
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/