Valdis,
On Thu, 8 Aug 2019, Valdis Klētnieks wrote:
I really appreciate your work, but can you please refrain from using file
names as prefixes?
git log $FILE gives you usually a pretty good hint what the proper prefix
is:
bd9a0c97e53c ("x86/umwait: Add sysfs interface to control umwait maximum
time")
ff4b353f2ef9 ("x86/umwait: Add sysfs interface to control umwait C0.2 state")
bd688c69b7e6 ("x86/umwait: Initialize umwait control values")
See?
> We get a warning when building with W=1:
Please avoid 'We/I' in changelogs.
> CC arch/x86/kernel/cpu/umwait.o
> arch/x86/kernel/cpu/umwait.c: In function 'umwait_init':
> arch/x86/kernel/cpu/umwait.c:183:6: warning: variable 'ret' set but not used
> [-Wunused-but-set-variable]
> 183 | int ret;
> | ^~~
>
> And indeed, we don't do anything with it, so clean it up.
Well, the question is whether removing the variable is the right thing to
do.
> Signed-off-by: Valdis Kletnieks <[email protected]>
>
> diff --git a/arch/x86/kernel/cpu/umwait.c b/arch/x86/kernel/cpu/umwait.c
> index 6a204e7336c1..3d1d3952774a 100644
> --- a/arch/x86/kernel/cpu/umwait.c
> +++ b/arch/x86/kernel/cpu/umwait.c
> @@ -180,12 +180,11 @@ static struct attribute_group umwait_attr_group = {
> static int __init umwait_init(void)
> {
> struct device *dev;
> - int ret;
>
> if (!boot_cpu_has(X86_FEATURE_WAITPKG))
> return -ENODEV;
>
> - ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "umwait:online",
> + cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "umwait:online",
> umwait_cpu_online, NULL);
If that fails then umwait is broken. So instead of removing it, this should
actually check the return code and act accordingly. Fenghua?
> register_syscore_ops(&umwait_syscore_ops);
Thanks,
tglx
