On 9/11/20 1:23 PM, Ard Biesheuvel wrote:
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.
Thanks, I have taken care of the comments and sent out the next revision now.
https://lkml.kernel.org/r/20200903234531.162484-2-Smita.KoralahalliChannabasappa@xxxxxxx.
Please ignore my previous email.
Thanks,
Smita
