Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 27 Oct 2021 13:41:03 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Cc:     x86@kernel.org, linux-edac@vger.kernel.org,
        linux-kernel@vger.kernel.org, Tony Luck <tony.luck@intel.com>,
        "H . Peter Anvin" <hpa@zytor.com>, yazen.ghannam@amd.com
Subject: Re: [PATCH v2 3/5] x86/mce: Use mca_msr_reg() in prepare_msrs()
Message-ID: <YXk6z9xWvS4B7eRP@zn.tnic>
References: <20211019233641.140275-1-Smita.KoralahalliChannabasappa@amd.com>
 <20211019233641.140275-4-Smita.KoralahalliChannabasappa@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20211019233641.140275-4-Smita.KoralahalliChannabasappa@amd.com>
Precedence: bulk

On Tue, Oct 19, 2021 at 06:36:39PM -0500, Smita Koralahalli wrote:
> Replace MCx_{STATUS, ADDR, MISC} macros with mca_msr_reg().

And this is where your commit message and patch should end. It is a bad
idea to do textual replacements *and* functional changes in a single
patch: it is hard to review and debug if there are possible issues. So
you do the textual replacements in the first one and then the functional
changes in subsequent patches.

> Also, restructure the code to avoid multiple initializations for MCA
> registers.

What multiple initializations?

> SMCA machines define a different set of MSRs for MCA registers
> and mca_msr_reg() returns the proper MSR address for SMCA and legacy
> processors.
> 
> Initialize MCA_MISC and MCA_SYND registers at the end after initializing
> MCx_{STATUS, DESTAT} which is further explained in the next patch.

And this should be *in* the next patch.

Also, there's no concept of "next patch" when you do git log on the
upstream tree and use different sorting etc. So a patch should be
self-contained and do one change only.

There's very good documentation in Documentation/process/, expecially
Documentation/process/submitting-patches.rst, which explains how a patch
should look like.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette