| Message ID | 20191027155410.187957-16-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,01/30] lib: Allow crc32 to be disabled. | expand |
On 10/27/19 4:53 PM, Simon Glass wrote:> Two files relay on efi_driver.h to include common.h and dm.h which is %s/relay/rely/ > incorrect. The former should always be included in a non-host C file and > the latter should be included if driver model is used. https://www.denx.de/wiki/U-Boot/CodingStyle does no mention these requirements. Does the wiki need an update? Non-host C files not including common.h at all or not as the first include call for trouble, cf. https://lists.denx.de/pipermail/u-boot/2019-October/388406.html https://lists.denx.de/pipermail/u-boot/2019-October/388408.html As your patch makes it obvious that common.h is included first: Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > include/efi_driver.h | 2 -- > lib/efi_driver/efi_block_device.c | 2 ++ > lib/efi_driver/efi_uclass.c | 2 ++ > 3 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/efi_driver.h b/include/efi_driver.h > index 840483a416..2b62219c5b 100644 > --- a/include/efi_driver.h > +++ b/include/efi_driver.h > @@ -8,8 +8,6 @@ > #ifndef _EFI_DRIVER_H > #define _EFI_DRIVER_H 1 > > -#include <common.h> > -#include <dm.h> > #include <efi_loader.h> > > /* > diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c > index cf02341931..c7e7946cdd 100644 > --- a/lib/efi_driver/efi_block_device.c > +++ b/lib/efi_driver/efi_block_device.c > @@ -28,6 +28,8 @@ > * iPXE uses the simple file protocol to load Grub or the Linux Kernel. > */ > > +#include <common.h> > +#include <dm.h> > #include <efi_driver.h> > #include <dm/device-internal.h> > #include <dm/root.h> > diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c > index b14746e6b1..c837db165c 100644 > --- a/lib/efi_driver/efi_uclass.c > +++ b/lib/efi_driver/efi_uclass.c > @@ -17,6 +17,8 @@ > * controllers. > */ > > +#include <common.h> > +#include <dm.h> > #include <efi_driver.h> > > /** >
Hi Heinrich, On Sun, 27 Oct 2019 at 11:42, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 10/27/19 4:53 PM, Simon Glass wrote:> Two files relay on efi_driver.h > to include common.h and dm.h which is > %s/relay/rely/ > > > incorrect. The former should always be included in a non-host C file and > > the latter should be included if driver model is used. > > https://www.denx.de/wiki/U-Boot/CodingStyle does no mention these > requirements. Does the wiki need an update? OK I added a bit of an update. Feel free to improve it. > > Non-host C files not including common.h at all or not as the first > include call for trouble, cf. > > https://lists.denx.de/pipermail/u-boot/2019-October/388406.html > https://lists.denx.de/pipermail/u-boot/2019-October/388408.html > > As your patch makes it obvious that common.h is included first: > > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > Thanks...at some point we might consider running a tool over the code base to sort the includes. Regards, Simon
diff --git a/include/efi_driver.h b/include/efi_driver.h index 840483a416..2b62219c5b 100644 --- a/include/efi_driver.h +++ b/include/efi_driver.h @@ -8,8 +8,6 @@ #ifndef _EFI_DRIVER_H #define _EFI_DRIVER_H 1 -#include <common.h> -#include <dm.h> #include <efi_loader.h> /* diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index cf02341931..c7e7946cdd 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -28,6 +28,8 @@ * iPXE uses the simple file protocol to load Grub or the Linux Kernel. */ +#include <common.h> +#include <dm.h> #include <efi_driver.h> #include <dm/device-internal.h> #include <dm/root.h> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index b14746e6b1..c837db165c 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -17,6 +17,8 @@ * controllers. */ +#include <common.h> +#include <dm.h> #include <efi_driver.h> /**
Two files relay on efi_driver.h to include common.h and dm.h which is incorrect. The former should always be included in a non-host C file and the latter should be included if driver model is used. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/efi_driver.h | 2 -- lib/efi_driver/efi_block_device.c | 2 ++ lib/efi_driver/efi_uclass.c | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-)