Jukka Ruohonen wrote:
> +If the backend operates with a simple boolean switch
> +without knowing the clock frequencies, the
> +.Fa cfs_freq
> +field should be set to
> +.Dv CPUFREQ_STATE_ENABLED
> +or
> +.Dv CPUFREQ_STATE_DISABLED .
Elements must go in descendant order but since values of
CPUFREQ_STATE_DISABLED and CPUFREQ_STATE_ENABLED are unspecified,
the manual should explicitly state in which order these two states
should go.
> + /*
> + * Sanity check the values and verify descending order.
> + */
> + for (count = i = 0; i < cf->cf_state_count; i++) {
> +
> + CTASSERT(CPUFREQ_STATE_ENABLED != 0);
> + CTASSERT(CPUFREQ_STATE_DISABLED != 0);
> +
> + if (cf->cf_state[i].cfs_freq == 0)
> + continue;
Every time you skip cf->cf_state element, you have a gap in the
corresponding cf_state element in cf_backend. It's zeroed (because
you initialized cf_backend with kmem_zalloc) but I doubt is was
your intention.
The documentation is clear about cf_state elements, why do you need
to skip entries at all and make your life harder? Just return error
or add KASSERT if a caller doesn't follow the docs.
> + for (j = k = 0; j < i; j++) {
> +
> + if (cf->cf_state[i].cfs_freq >=
> + cf->cf_state[j].cfs_freq) {
> + k = 1;
> + break;
> + }
> + }
If you didn't skip elements, you could check only the previous element
rather than doing it in a loop
> + if (k != 0)
> + continue;
... and return error rather than skipping an out-of-order element.
> + cf_backend->cf_state[i].cfs_index = count;
> + cf_backend->cf_state[i].cfs_freq = cf->cf_state[i].cfs_freq;
> + cf_backend->cf_state[i].cfs_power = cf->cf_state[i].cfs_power;
^
Alternatively, you can change i here ^ to count to avoid gaps.
Hmm, cf_index assignment above makes me wonder that may be gaps are on
purpose but not documented?
Alex
