From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751883AbcBWMfm (ORCPT ); Tue, 23 Feb 2016 07:35:42 -0500 Received: from mail.skyhub.de ([78.46.96.112]:57495 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751457AbcBWMfl (ORCPT ); Tue, 23 Feb 2016 07:35:41 -0500 Date: Tue, 23 Feb 2016 13:35:30 +0100 From: Borislav Petkov To: Aravind Gopalakrishnan Cc: tony.luck@intel.com, hpa@zytor.com, mingo@redhat.com, tglx@linutronix.de, dougthompson@xmission.com, mchehab@osg.samsung.com, x86@kernel.org, linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org, ashok.raj@intel.com, gong.chen@linux.intel.com, len.brown@intel.com, peterz@infradead.org, ak@linux.intel.com, alexander.shishkin@linux.intel.com Subject: Re: [PATCH 4/4] x86/mce/AMD: Add comments for easier understanding Message-ID: <20160223123530.GB3673@pd.tnic> References: <1455659111-32074-1-git-send-email-Aravind.Gopalakrishnan@amd.com> <1455659111-32074-5-git-send-email-Aravind.Gopalakrishnan@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1455659111-32074-5-git-send-email-Aravind.Gopalakrishnan@amd.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 16, 2016 at 03:45:11PM -0600, Aravind Gopalakrishnan wrote: > In an attempt to help folks not very familiar with the code to > understand what the code is doing, adding a bit of helper > comments around some more important functions in the driver > to describe them. > > No functional change is introduced. > > Signed-off-by: Aravind Gopalakrishnan > --- > arch/x86/kernel/cpu/mcheck/mce_amd.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c > index 4bdc836..d2b6001 100644 > --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c > +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c > @@ -184,6 +184,11 @@ static int lvt_off_valid(struct threshold_block *b, int apic, u32 lo, u32 hi) > }; > > /* > + * Set the error_count and interrupt_enable sysfs attributes here. > + * This function gets called during the init phase and when someone > + * makes changes to either of the sysfs attributes. > + * During init phase, we also program Interrupt type as 'APIC' and > + * verify if LVT offset obtained from MCx_MISC is valid. > * Called via smp_call_function_single(), must be called with correct > * cpu affinity. > */ I don't think that's what threshold_restart_bank() does... Also, that comment is too much - it shouldn't explain "what" but "why". > @@ -262,6 +267,11 @@ static int setup_APIC_deferred_error(int reserved, int new) > return reserved; > } > > +/* > + * Obtain LVT offset from MSR_CU_DEF_ERR and call > + * setup_APIC_deferred_error() to program relevant APIC register. > + * Also, register a deferred error interrupt handler > + */ No, that's basically spelling what the code does. > static void deferred_error_interrupt_enable(struct cpuinfo_x86 *c) > { > u32 low = 0, high = 0; > @@ -338,6 +348,14 @@ nextaddr_out: > return addr; > } > > +/* > + * struct threshold_block descriptor tracks useful info regarding the > + * banks' MISC register. Among other things, it tracks whether interrupt > + * is possible for the given bank, the threshold limit and the sysfs object > + * that outputs these info. That should be in form of comments explaining what the members of struct threshold_block are, where that struct is defined. > Initializing the struct here, programming > + * LVT offset for threshold interrupts and registering a interrupt handler > + * if we haven't already done so Also spelling the code. > + */ > static int > prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr, > int offset, u32 misc_high) -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply.