Re: [Xen-devel] [RFC PATCH v2 11/12] xen: arm: implement XEN_SYSCTL_cpufreq_op

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.10.14 at 13:27, <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
> +int sysctl_cpufreq_op(xen_sysctl_cpufreq_op_t *op)
> +{
> +    int ret = 0;
> +    switch ( op->cmd )
> +    {
> +    case XEN_SYSCTL_CPUFREQ_get_target:
> +        spin_lock(&sysctl_cpufreq_lock);
> +        op->u.target.cpu = sysctl_cpufreq_data.cpu;
> +        op->u.target.freq = sysctl_cpufreq_data.freq;
> +        op->u.target.relation = sysctl_cpufreq_data.relation;
> +        spin_unlock(&sysctl_cpufreq_lock);
> +        break;
> +
> +    case XEN_SYSCTL_CPUFREQ_set_result:
> +        spin_lock(&sysctl_cpufreq_lock);
> +        sysctl_cpufreq_data.result = op->u.result;
> +        spin_unlock(&sysctl_cpufreq_lock);
> +        break;

Seeing this and taking it together with the previous patch I still
can't really see how all this is intended to fit together, and
where the frequency change really takes place. I'm particularly
lacking understanding of how the result of the operation is
intended to be made available for the right .target handler
invocation - right now it looks like the next invocation will get
to see the result of the previous one.

> +    default:
> +        return -ENOSYS;

-EINVAL or -EOPNOTSUPP please.

> --- a/xen/include/xen/cpufreq.h
> +++ b/xen/include/xen/cpufreq.h
> @@ -20,6 +20,7 @@
>  #include <xen/spinlock.h>
>  #include <xen/errno.h>
>  #include <xen/cpumask.h>
> +#include <public/sysctl.h>

Please don't add new dependencies that aren't needed (not even
on ARM).

> @@ -264,4 +265,8 @@ int write_userspace_scaling_setspeed(unsigned int cpu, 
> unsigned int freq);
>  void cpufreq_dbs_timer_suspend(void);
>  void cpufreq_dbs_timer_resume(void);
>  
> +#ifdef HAS_DOM0_CPUFREQ
> +int sysctl_cpufreq_op(xen_sysctl_cpufreq_op_t *op);
> +#endif

Simply forward declare and use struct xen_sysctl_cpufreq_op here.

And framing a declaration in #ifdef is relatively pointless too.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel