On Thu, Jun 15, 2023 at 9:21 AM Jan Beulich <jbeul...@suse.com> wrote:
>
> On 14.06.2023 20:02, Jason Andryuk wrote:
> > @@ -121,6 +122,12 @@ int __init cpufreq_register_governor(struct
> > cpufreq_governor *governor)
> > if (!governor)
> > return -EINVAL;
> >
> > + if (cpufreq_governor_internal && !governor->internal)
> > + return -EINVAL;
> > +
> > + if (!cpufreq_governor_internal && governor->internal)
> > + return -EINVAL;
>
> First just a nit: Why not simply
>
> if (cpufreq_governor_internal != governor->internal)
> return -EINVAL;
>
> ?
Yes, that would work.
> Yet then I find this approach a little odd anyway. I think the
> registration attempts would better be suppressed, thus also not
> resulting in (apparently) failed init-calls. Especially for the
> userspace governor this would then also mean / allow to avoid
> registering of the CPU notifier.
So are you suggesting that each caller check cpufreq_governor_internal
and potentially skip calling cpufreq_register_governor()? e.g. the
start of cpufreq_gov_userspace_init() would gain:
if (cpufreq_governor_internal)
return 0
?
I put the check in cpufreq_register_governor() since then it is
handled in a single place instead of being spread around.
To leave the check in cpufreq_register_governor(),
cpufreq_gov_userspace_init() could be rearranged to call
cpufreq_register_governor() first and only call
register_cpu_notifier() on success?
Or do you want the whole governor registration to be re-worked to be
more explicit? Remove initcalls and have governors registered
according to the cpufreq driver?
Regards,
Jason
