Re: [PATCH v2] platform/cpufreq: add public defines for CPUFREQ_SHARED_TYPE_

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Apr 07, 2022 at 10:48:50AM +0200, Jan Beulich wrote:
> On 07.04.2022 10:18, Roger Pau Monne wrote:
> > The values set in the shared_type field of xen_processor_performance
> > have so far relied on Xen and Linux having the same
> > CPUFREQ_SHARED_TYPE_ defines, as those have never been part of the
> > public interface.
> > 
> > Formalize by adding the defines for the allowed values in the public
> > header, while renaming them to use the XEN_CPUPERF_SHARED_TYPE_ prefix
> > for clarity.
> > 
> > Set the Xen internal defines for CPUFREQ_SHARED_TYPE_ using the newly
> > introduced XEN_CPUPERF_SHARED_TYPE_ public defines in order to avoid
> > unnecessary code churn.  While there also drop
> > CPUFREQ_SHARED_TYPE_NONE as it's unused.
> > 
> > Fixes: 2fa7bee0a0 ('Get ACPI Px from dom0 and choose Px controller')
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> with one remark:
> 
> > --- a/xen/include/acpi/cpufreq/cpufreq.h
> > +++ b/xen/include/acpi/cpufreq/cpufreq.h
> > @@ -78,10 +78,9 @@ DECLARE_PER_CPU(struct cpufreq_policy *, 
> > cpufreq_cpu_policy);
> >  extern int __cpufreq_set_policy(struct cpufreq_policy *data,
> >                                  struct cpufreq_policy *policy);
> >  
> > -#define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
> 
> I realize this is unused, but do we really want/need to drop this?
> I think it is used implicitly right now by assuming the value would
> be zero; this could do with making explicit, in which case we'd
> need the #define.

I don't think Xen uses it explicitly, all checks of shared_type are
always against a specific CPUFREQ_SHARED_TYPE_{HW,ALL,ANY}.

I don't have a strong opinion about keeping it however, I've just
removed as a cleanup, if you don't think it's helpful I can resubmit
keeping the define.

Thanks, Roger.