[Public]
Hi,
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Tuesday, April 29, 2025 7:47 PM
> To: Penny, Zheng <penny.zh...@amd.com>
> Cc: Huang, Ray <ray.hu...@amd.com>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v4 04/15] xen/cpufreq: refactor cmdline "cpufreq=xxx"
>
> On 29.04.2025 12:36, Jan Beulich wrote:
> > On 14.04.2025 09:40, Penny Zheng wrote:
> >> --- a/xen/drivers/cpufreq/cpufreq.c
> >> +++ b/xen/drivers/cpufreq/cpufreq.c
> >> @@ -71,6 +71,49 @@ unsigned int __initdata cpufreq_xen_cnt = 1;
> >>
> >> static int __init cpufreq_cmdline_parse(const char *s, const char
> >> *e);
> >>
> >> +static bool __init cpufreq_opts_contain(enum cpufreq_xen_opt option)
> >> +{
> >> + unsigned int count = cpufreq_xen_cnt;
> >> +
> >> + while ( count )
> >> + {
> >> + if ( cpufreq_xen_opts[--count] == option )
> >> + return true;
> >> + }
> >> +
> >> + return false;
> >> +}
> >> +
> >> +static int __init handle_cpufreq_cmdline(enum cpufreq_xen_opt
> >> +option) {
> >> + int ret = 0;
> >> +
> >> + if ( cpufreq_opts_contain(option) )
> >> + {
> >> + const char *cpufreq_opts_str[] = { "CPUFREQ_xen",
> >> + "CPUFREQ_hwp" };
> >
> > const char *const __initconstrel cpufreq_opts_str[] = {
> > "CPUFREQ_xen", "CPUFREQ_hwp" };
> >
> > (line wrapped suitably, of course) Or maybe even better
> >
> > const char __initconst cpufreq_opts_str[][12] = {
> > "CPUFREQ_xen", "CPUFREQ_hwp" };
> >
> > for the string literals to also end up in .init.rodata.
>
> Actually, it didn't even occur to me that there might be a "static" missing
> here, too.
Sorry, I may need more explanation, what is the "static" missing you are
referring?
> Plus I'm missing any arrangement for the array slots to remain in sync with
> the
> corresponding enumerators. With that ...
>
Thanks for the detailed instructions, learned and I'll change it to
const char __initconst cpufreq_opts_str[][4] = { "xen", "hwp" };
And for in sync with the corresponding enumerators, maybe we shall add comment
here,
/* The order of cpufreq string literal must be in sync with the order
in "enum cpufreq_xen_opt" */
> > With all of the adjustments:
> > Reviewed-by: Jan Beulich <jbeul...@suse.com>
>
> I'm sorry, but I need to take this back. There are just too many issues.
>
> Jan
