On Thu, 3 Sep 2020 at 09:34, Punit Agrawal <punit1.agrawal@xxxxxxxxxxxxx> wrote: > > Hi Smita, > > Smita Koralahalli Channabasappa <skoralah@xxxxxxx> writes: > > > On 8/31/20 12:05 AM, Punit Agrawal wrote: > > > >> Hi Smita, > >> > >> A couple of comments below - > >> > >> Smita Koralahalli <Smita.KoralahalliChannabasappa@xxxxxxx> writes: > >> > >> [...] > >> > >> > >>> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c > >>> index 2531de49f56c..374b8e18552a 100644 > >>> --- a/drivers/firmware/efi/cper-x86.c > >>> +++ b/drivers/firmware/efi/cper-x86.c > >>> @@ -1,7 +1,7 @@ > >>> // SPDX-License-Identifier: GPL-2.0 > >>> // Copyright (C) 2018, Advanced Micro Devices, Inc. > >>> -#include <linux/cper.h> > >> Why is the include dropped? AFAICT, the definitions from there are still > >> being used after this patch. > > > > Dropped because <acpi/apei.h> already includes <linux/cper.h> > > Generally, you want to follow the rule that if a declaration from a > header file is being used, it should show up in the includes. The same > applies to both source as well as header files. > > It doesn't matter if another include in the source file in turn ends up > including the same header again; the #ifdef guards are there to prevent > duplicate declarations. > > The rationale is that if future changes remove the usage of > <acpi/apei.h>, the C file can still be compiled after dropping the > include; there should be no need to then re-introduce <linux/cper.h> at > that point. > > Hope that makes sense. > Agreed. If the code still uses declarations from linux/cper.h after the patch, the #include should remain.
