Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754496AbbGQUjo (ORCPT ); Fri, 17 Jul 2015 16:39:44 -0400 Received: from www.linutronix.de ([62.245.132.108]:48961 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754365AbbGQUjm (ORCPT ); Fri, 17 Jul 2015 16:39:42 -0400 Date: Fri, 17 Jul 2015 22:39:25 +0200 (CEST) From: Thomas Gleixner To: Laura Abbott cc: Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Peter Zijlstra , Borislav Petkov , Andy Lutomirski Subject: Re: [PATCHv2] x86, CPU: Restore MSR_IA32_ENERGY_PERF_BIAS after resume In-Reply-To: <1437157964-19389-1-git-send-email-labbott@fedoraproject.org> Message-ID: References: <1437157964-19389-1-git-send-email-labbott@fedoraproject.org> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2023 Lines: 67 On Fri, 17 Jul 2015, Laura Abbott wrote: > v2: Tweaked a few names to be more descriptive Descriptive by some definition of descriptive. See below. > +static void init_intel_energy_perf(struct cpuinfo_x86 *c) > +{ > + /* > + * Initialize MSR_IA32_ENERGY_PERF_BIAS if BIOS did not. > + * x86_energy_perf_policy(8) is available to change it at run-time > + */ > + if (cpu_has(c, X86_FEATURE_EPB)) { Make this if (!cpu_has(c, X86_FEATURE_EPB)) return; and spare the extra indentation level. > + u64 epb; > + rdmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb); > + if ((epb & 0xF) == ENERGY_PERF_BIAS_PERFORMANCE) { Ditto > + pr_warn_once("ENERGY_PERF_BIAS: Set to 'normal', was 'performance'\n"); > + pr_warn_once("ENERGY_PERF_BIAS: View and update with x86_energy_perf_policy(8)\n"); > + epb = (epb & ~0xF) | ENERGY_PERF_BIAS_NORMAL; > + wrmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb); > + } > + } > +} > #ifdef CONFIG_X86_32 > @@ -747,6 +752,7 @@ static const struct cpu_dev intel_cpu_dev = { > .c_detect_tlb = intel_detect_tlb, > .c_early_init = early_init_intel, > .c_init = init_intel, > + .c_bsp_resume = init_intel_energy_perf, Looking at the resulting code I have no idea WHY init_intel_energy_perf is set here. So much for descriptive. This really wants to be bsp_resume() or something like this (add the pointless intel prefix if it makes your managers happy). That _IS_ actually descriptive. static void bsp_resume(struct cpuinfo_x86 *c) { /* * Some reasonable comment WHY we call this here. */ init_intel_energy_perf(c); } Aside of documenting what that resume thing is for and why init_intel_energy_perf() needs to be called, I'm quite sure that this will fill up pretty fast with other stuff which gets lost across S/R. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/