Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On Thu, 16 Dec 2021 23:09:03 +0100 Marek Behún wrote: > On Thu, 16 Dec 2021 19:16:40 +0100 > Pali Rohár wrote: > > > The reason that it currently works is just because > > gcc compiler does not do all checks before doing optimizations and so it > > currently does generate any errors or warnings. > > Compiler cannot currently check this, only linker, because the function > is always declared in mvebu's cpu.h. > > See https://lore.kernel.org/u-boot/20211214134536.2baeb2a0@thinkpad/ > where I also proposed empty static inline implementations for non-A375 > platforms, but Stefan thinks it's not an issue currently, because it > does not cause any regressions, I guess. U-Boot's build system > currently does not allow for -O0, you can choose only -O2 or -Os. > > We can always add empty static inline implementations into mvebu's > cpu.h when it becomes an issue, or you can send a patch now, if you > want a completely perfect code ASAP. > > But note that for that you'll need to check other functions there, as > well. (If you look at > > https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-mvebu/include/mach/cpu.h > there are functions declared, without guarding #ifs, for all mvebu > platforms: A3k, A7k, A37x and A38x.) And btw, I just tried forcing -O0, and didn't even get to SPL compiling stage. U-Boot proper didn't failed to link with: undefined reference to `of_read_u32_index' undefined reference to `of_read_u64' undefined reference to `of_find_property' undefined reference to `of_read_u32_array' undefined reference to `of_device_is_available' undefined reference to `of_get_parent' undefined reference to `of_get_address' undefined reference to `of_n_size_cells' undefined reference to `of_translate_address' undefined reference to `of_n_addr_cells' undefined reference to `of_property_match_string' undefined reference to `of_parse_phandle_with_args' undefined reference to `of_count_phandle_with_args' undefined reference to `of_find_node_opts_by_path' undefined reference to `of_get_property' undefined reference to `of_device_is_available' undefined reference to `of_get_property' undefined reference to `of_simple_addr_cells' undefined reference to `of_simple_size_cells' undefined reference to `of_device_is_compatible' undefined reference to `of_get_stdout' Since no-one noticed this till now, I would bet the reality is that -O0 really isn't done, and if someone really needs it, they will have to fix other things as well. Also with -O0 I think SPL would be too big so you won't be able to test it anyway. Although you could study generated and linked assembler code, but why would you do that? You can just disassemble the object file. Marek
Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On Thu, 16 Dec 2021 19:16:40 +0100 Pali Rohár wrote: > The reason that it currently works is just because > gcc compiler does not do all checks before doing optimizations and so it > currently does generate any errors or warnings. Compiler cannot currently check this, only linker, because the function is always declared in mvebu's cpu.h. See https://lore.kernel.org/u-boot/20211214134536.2baeb2a0@thinkpad/ where I also proposed empty static inline implementations for non-A375 platforms, but Stefan thinks it's not an issue currently, because it does not cause any regressions, I guess. U-Boot's build system currently does not allow for -O0, you can choose only -O2 or -Os. We can always add empty static inline implementations into mvebu's cpu.h when it becomes an issue, or you can send a patch now, if you want a completely perfect code ASAP. But note that for that you'll need to check other functions there, as well. (If you look at https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-mvebu/include/mach/cpu.h there are functions declared, without guarding #ifs, for all mvebu platforms: A3k, A7k, A37x and A38x.) Marek
Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On Tuesday 14 December 2021 14:01:04 Marek Behún wrote:
> On Tue, 14 Dec 2021 13:48:31 +0100
> Stefan Roese wrote:
>
> > On 12/14/21 13:45, Marek Behún wrote:
> > > On Tue, 14 Dec 2021 12:12:34 +0100
> > > Pali Rohár wrote:
> > >
> > >> On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:
> > >>> On Tue, 14 Dec 2021 10:36:00 +0100
> > >>> Pali Rohár wrote:
> > >>>
> > On Friday 26 November 2021 15:37:37 Marek Behún wrote:
> > > @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> > > timer_init();
> > >
> > > /* Armada 375 does not support SerDes and DDR3 init yet */
> > > -#if !defined(CONFIG_ARMADA_375)
> > > - /* First init the serdes PHY's */
> > > - serdes_phy_config();
> > > -
> > > - /* Setup DDR */
> > > - ret = ddr3_init();
> > > - if (ret) {
> > > - debug("ddr3_init() failed: %d\n", ret);
> > > - hang();
> > > + if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> > > + /* First init the serdes PHY's */
> > > + serdes_phy_config();
> > > +
> > > + /* Setup DDR */
> > > + ret = ddr3_init();
> > > + if (ret) {
> > > + debug("ddr3_init() failed: %d\n", ret);
> > > + hang();
> > > + }
> > > }
> > > -#endif
> >
> > As written in comment above there is no SerDes and DDR3 support for
> > Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> > function. So this code needs not to be compiled at all and usage of
> > #ifdef is correct here.
> > >>>
> > >>> #ifdefs are almost never correct in C-files, for the parts of the code
> > >>> they guard isn't put through syntactic analysis, and can therefore
> > >>> contain bugs which we are not warned about.
> > >>>
> > >>> Using if (IS_ENABLED()) almost never producess a different binary,
> > >>> because the code is optimized away.
> > >>>
> > >>> Marek
> > >>
> > >> There is no function serdes_phy_config() for Armada 375, so if you put
> > >> it outside of #ifdef you will get compile error.
> > >
> > > The function is always declared in
> > >arch/arm/mach-mvebu/include/mach/cpu.h
> > > regardless of architecture.
> > >
> > > Thus an error will be raised only when linking, and the compliation was
> > > done with -O0, which I don't think anyone does.
> > >
> > > Anyway, if we want to support -O0, this can and should be solved via
> > > defining serdes_phy_config() as empty static inline function in the
> > > cpu.h header, guarded by #ifdef. In header files #ifdefs are allowed,
> > > in this manner:
> > >#if X
> > >declare function
> > >#else
> > >define that function as empty static inline
> > >#endif
> > >
> > > So if you think we should support -O0, I can do this.
> > >
> > > But the #ifdefs should really go away from real C code, that is the way
> > > both Linux and U-Boot are trying to go for the last couple of years, if
> > > I understand it correctly.
> >
> > Yes, the #ifdef's really should be avoided if possible. So *if* your
> > patch above does not generate any build issues, then I don't see any
> > problems to include it. I personally don't think that we need to support
> > -O0 builds.
>
> db-88f6720_defconfig builds without issue (armada 375). And it builds the
> relevant file, spl/arch/arm/mach-mvebu/spl.o.
>
> Marek
-O0 is useful for debugging purposes, it generates more readable
assembler code.
Anyway, the issue here is that those two functions are not defined and
implemented for armada 375 soc. #ifdef is here to selectively do not
compile code which is not implemented on armada 375. And this cannot be
done by normal if(). The reason that it currently works is just because
gcc compiler does not do all checks before doing optimizations and so it
currently does generate any errors or warnings. But this is just
undefined behavior and like any other undefined behavior it may change
in some future version of gcc (or changing compiler to some other).
This approach with converting correct #ifdef to if() with undefined
behavior just hides the real issue that those two functions are not
defined and implemented for all mvebu platforms.
Why not rather to define these two functions are empty static inline
stubs with big comment that they are missing? I think this is proper
solution as it does not depends on undefined behavior of compiler and
linker, supports also -O0 and removes that #ifdef in spl.c file.
Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On Tue, 14 Dec 2021 13:48:31 +0100
Stefan Roese wrote:
> On 12/14/21 13:45, Marek Behún wrote:
> > On Tue, 14 Dec 2021 12:12:34 +0100
> > Pali Rohár wrote:
> >
> >> On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:
> >>> On Tue, 14 Dec 2021 10:36:00 +0100
> >>> Pali Rohár wrote:
> >>>
> On Friday 26 November 2021 15:37:37 Marek Behún wrote:
> > @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> > timer_init();
> >
> > /* Armada 375 does not support SerDes and DDR3 init yet */
> > -#if !defined(CONFIG_ARMADA_375)
> > - /* First init the serdes PHY's */
> > - serdes_phy_config();
> > -
> > - /* Setup DDR */
> > - ret = ddr3_init();
> > - if (ret) {
> > - debug("ddr3_init() failed: %d\n", ret);
> > - hang();
> > + if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> > + /* First init the serdes PHY's */
> > + serdes_phy_config();
> > +
> > + /* Setup DDR */
> > + ret = ddr3_init();
> > + if (ret) {
> > + debug("ddr3_init() failed: %d\n", ret);
> > + hang();
> > + }
> > }
> > -#endif
>
> As written in comment above there is no SerDes and DDR3 support for
> Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> function. So this code needs not to be compiled at all and usage of
> #ifdef is correct here.
> >>>
> >>> #ifdefs are almost never correct in C-files, for the parts of the code
> >>> they guard isn't put through syntactic analysis, and can therefore
> >>> contain bugs which we are not warned about.
> >>>
> >>> Using if (IS_ENABLED()) almost never producess a different binary,
> >>> because the code is optimized away.
> >>>
> >>> Marek
> >>
> >> There is no function serdes_phy_config() for Armada 375, so if you put
> >> it outside of #ifdef you will get compile error.
> >
> > The function is always declared in
> >arch/arm/mach-mvebu/include/mach/cpu.h
> > regardless of architecture.
> >
> > Thus an error will be raised only when linking, and the compliation was
> > done with -O0, which I don't think anyone does.
> >
> > Anyway, if we want to support -O0, this can and should be solved via
> > defining serdes_phy_config() as empty static inline function in the
> > cpu.h header, guarded by #ifdef. In header files #ifdefs are allowed,
> > in this manner:
> >#if X
> >declare function
> >#else
> >define that function as empty static inline
> >#endif
> >
> > So if you think we should support -O0, I can do this.
> >
> > But the #ifdefs should really go away from real C code, that is the way
> > both Linux and U-Boot are trying to go for the last couple of years, if
> > I understand it correctly.
>
> Yes, the #ifdef's really should be avoided if possible. So *if* your
> patch above does not generate any build issues, then I don't see any
> problems to include it. I personally don't think that we need to support
> -O0 builds.
db-88f6720_defconfig builds without issue (armada 375). And it builds the
relevant file, spl/arch/arm/mach-mvebu/spl.o.
Marek
Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On 12/14/21 13:45, Marek Behún wrote:
On Tue, 14 Dec 2021 12:12:34 +0100
Pali Rohár wrote:
On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:
On Tue, 14 Dec 2021 10:36:00 +0100
Pali Rohár wrote:
On Friday 26 November 2021 15:37:37 Marek Behún wrote:
@@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
timer_init();
/* Armada 375 does not support SerDes and DDR3 init yet */
-#if !defined(CONFIG_ARMADA_375)
- /* First init the serdes PHY's */
- serdes_phy_config();
-
- /* Setup DDR */
- ret = ddr3_init();
- if (ret) {
- debug("ddr3_init() failed: %d\n", ret);
- hang();
+ if (!IS_ENABLED(CONFIG_ARMADA_375)) {
+ /* First init the serdes PHY's */
+ serdes_phy_config();
+
+ /* Setup DDR */
+ ret = ddr3_init();
+ if (ret) {
+ debug("ddr3_init() failed: %d\n", ret);
+ hang();
+ }
}
-#endif
As written in comment above there is no SerDes and DDR3 support for
Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
function. So this code needs not to be compiled at all and usage of
#ifdef is correct here.
#ifdefs are almost never correct in C-files, for the parts of the code
they guard isn't put through syntactic analysis, and can therefore
contain bugs which we are not warned about.
Using if (IS_ENABLED()) almost never producess a different binary,
because the code is optimized away.
Marek
There is no function serdes_phy_config() for Armada 375, so if you put
it outside of #ifdef you will get compile error.
The function is always declared in
arch/arm/mach-mvebu/include/mach/cpu.h
regardless of architecture.
Thus an error will be raised only when linking, and the compliation was
done with -O0, which I don't think anyone does.
Anyway, if we want to support -O0, this can and should be solved via
defining serdes_phy_config() as empty static inline function in the
cpu.h header, guarded by #ifdef. In header files #ifdefs are allowed,
in this manner:
#if X
declare function
#else
define that function as empty static inline
#endif
So if you think we should support -O0, I can do this.
But the #ifdefs should really go away from real C code, that is the way
both Linux and U-Boot are trying to go for the last couple of years, if
I understand it correctly.
Yes, the #ifdef's really should be avoided if possible. So *if* your
patch above does not generate any build issues, then I don't see any
problems to include it. I personally don't think that we need to support
-O0 builds.
Thanks,
Stefan
Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On Tue, 14 Dec 2021 12:12:34 +0100
Pali Rohár wrote:
> On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:
> > On Tue, 14 Dec 2021 10:36:00 +0100
> > Pali Rohár wrote:
> >
> > > On Friday 26 November 2021 15:37:37 Marek Behún wrote:
> > > > @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> > > > timer_init();
> > > >
> > > > /* Armada 375 does not support SerDes and DDR3 init yet */
> > > > -#if !defined(CONFIG_ARMADA_375)
> > > > - /* First init the serdes PHY's */
> > > > - serdes_phy_config();
> > > > -
> > > > - /* Setup DDR */
> > > > - ret = ddr3_init();
> > > > - if (ret) {
> > > > - debug("ddr3_init() failed: %d\n", ret);
> > > > - hang();
> > > > + if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> > > > + /* First init the serdes PHY's */
> > > > + serdes_phy_config();
> > > > +
> > > > + /* Setup DDR */
> > > > + ret = ddr3_init();
> > > > + if (ret) {
> > > > + debug("ddr3_init() failed: %d\n", ret);
> > > > + hang();
> > > > + }
> > > > }
> > > > -#endif
> > >
> > > As written in comment above there is no SerDes and DDR3 support for
> > > Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> > > function. So this code needs not to be compiled at all and usage of
> > > #ifdef is correct here.
> >
> > #ifdefs are almost never correct in C-files, for the parts of the code
> > they guard isn't put through syntactic analysis, and can therefore
> > contain bugs which we are not warned about.
> >
> > Using if (IS_ENABLED()) almost never producess a different binary,
> > because the code is optimized away.
> >
> > Marek
>
> There is no function serdes_phy_config() for Armada 375, so if you put
> it outside of #ifdef you will get compile error.
The function is always declared in
arch/arm/mach-mvebu/include/mach/cpu.h
regardless of architecture.
Thus an error will be raised only when linking, and the compliation was
done with -O0, which I don't think anyone does.
Anyway, if we want to support -O0, this can and should be solved via
defining serdes_phy_config() as empty static inline function in the
cpu.h header, guarded by #ifdef. In header files #ifdefs are allowed,
in this manner:
#if X
declare function
#else
define that function as empty static inline
#endif
So if you think we should support -O0, I can do this.
But the #ifdefs should really go away from real C code, that is the way
both Linux and U-Boot are trying to go for the last couple of years, if
I understand it correctly.
Marek
Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:
> On Tue, 14 Dec 2021 10:36:00 +0100
> Pali Rohár wrote:
>
> > On Friday 26 November 2021 15:37:37 Marek Behún wrote:
> > > @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> > > timer_init();
> > >
> > > /* Armada 375 does not support SerDes and DDR3 init yet */
> > > -#if !defined(CONFIG_ARMADA_375)
> > > - /* First init the serdes PHY's */
> > > - serdes_phy_config();
> > > -
> > > - /* Setup DDR */
> > > - ret = ddr3_init();
> > > - if (ret) {
> > > - debug("ddr3_init() failed: %d\n", ret);
> > > - hang();
> > > + if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> > > + /* First init the serdes PHY's */
> > > + serdes_phy_config();
> > > +
> > > + /* Setup DDR */
> > > + ret = ddr3_init();
> > > + if (ret) {
> > > + debug("ddr3_init() failed: %d\n", ret);
> > > + hang();
> > > + }
> > > }
> > > -#endif
> >
> > As written in comment above there is no SerDes and DDR3 support for
> > Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> > function. So this code needs not to be compiled at all and usage of
> > #ifdef is correct here.
>
> #ifdefs are almost never correct in C-files, for the parts of the code
> they guard isn't put through syntactic analysis, and can therefore
> contain bugs which we are not warned about.
>
> Using if (IS_ENABLED()) almost never producess a different binary,
> because the code is optimized away.
>
> Marek
There is no function serdes_phy_config() for Armada 375, so if you put
it outside of #ifdef you will get compile error.
Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On Tue, 14 Dec 2021 10:36:00 +0100
Pali Rohár wrote:
> On Friday 26 November 2021 15:37:37 Marek Behún wrote:
> > @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> > timer_init();
> >
> > /* Armada 375 does not support SerDes and DDR3 init yet */
> > -#if !defined(CONFIG_ARMADA_375)
> > - /* First init the serdes PHY's */
> > - serdes_phy_config();
> > -
> > - /* Setup DDR */
> > - ret = ddr3_init();
> > - if (ret) {
> > - debug("ddr3_init() failed: %d\n", ret);
> > - hang();
> > + if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> > + /* First init the serdes PHY's */
> > + serdes_phy_config();
> > +
> > + /* Setup DDR */
> > + ret = ddr3_init();
> > + if (ret) {
> > + debug("ddr3_init() failed: %d\n", ret);
> > + hang();
> > + }
> > }
> > -#endif
>
> As written in comment above there is no SerDes and DDR3 support for
> Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> function. So this code needs not to be compiled at all and usage of
> #ifdef is correct here.
#ifdefs are almost never correct in C-files, for the parts of the code
they guard isn't put through syntactic analysis, and can therefore
contain bugs which we are not warned about.
Using if (IS_ENABLED()) almost never producess a different binary,
because the code is optimized away.
Marek
Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On Friday 26 November 2021 15:37:37 Marek Behún wrote:
> @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> timer_init();
>
> /* Armada 375 does not support SerDes and DDR3 init yet */
> -#if !defined(CONFIG_ARMADA_375)
> - /* First init the serdes PHY's */
> - serdes_phy_config();
> -
> - /* Setup DDR */
> - ret = ddr3_init();
> - if (ret) {
> - debug("ddr3_init() failed: %d\n", ret);
> - hang();
> + if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> + /* First init the serdes PHY's */
> + serdes_phy_config();
> +
> + /* Setup DDR */
> + ret = ddr3_init();
> + if (ret) {
> + debug("ddr3_init() failed: %d\n", ret);
> + hang();
> + }
> }
> -#endif
As written in comment above there is no SerDes and DDR3 support for
Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
function. So this code needs not to be compiled at all and usage of
#ifdef is correct here.
Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
On 11/26/21 15:37, Marek Behún wrote:
From: Marek Behún
Use the preferred
if (IS_ENABLED(X))
instead of
#ifdef X
where possible.
There are still places where this is not possible or is more complicated
to convert in this file. Leave those be for now.
Signed-off-by: Marek Behún
Nice, thanks.
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
arch/arm/mach-mvebu/spl.c | 43 ---
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 97d7aea179..7dbe8eeba3 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -150,26 +150,24 @@ int spl_parse_board_header(struct spl_image_info
*spl_image,
return -EINVAL;
}
-#ifdef CONFIG_SPL_SPI_FLASH_SUPPORT
- if (bootdev->boot_device == BOOT_DEVICE_SPI &&
+ if (IS_ENABLED(CONFIG_SPL_SPI_FLASH_SUPPORT) &&
+ bootdev->boot_device == BOOT_DEVICE_SPI &&
mhdr->blockid != IBR_HDR_SPI_ID) {
printf("ERROR: Wrong blockid (%u) in SPI kwbimage\n",
mhdr->blockid);
return -EINVAL;
}
-#endif
-#ifdef CONFIG_SPL_SATA
- if (bootdev->boot_device == BOOT_DEVICE_SATA &&
+ if (IS_ENABLED(CONFIG_SPL_SATA) &&
+ bootdev->boot_device == BOOT_DEVICE_SATA &&
mhdr->blockid != IBR_HDR_SATA_ID) {
printf("ERROR: Wrong blockid (%u) in SATA kwbimage\n",
mhdr->blockid);
return -EINVAL;
}
-#endif
-#ifdef CONFIG_SPL_MMC
- if ((bootdev->boot_device == BOOT_DEVICE_MMC1 ||
+ if (IS_ENABLED(CONFIG_SPL_MMC) &&
+ (bootdev->boot_device == BOOT_DEVICE_MMC1 ||
bootdev->boot_device == BOOT_DEVICE_MMC2 ||
bootdev->boot_device == BOOT_DEVICE_MMC2_2) &&
mhdr->blockid != IBR_HDR_SDIO_ID) {
@@ -177,18 +175,16 @@ int spl_parse_board_header(struct spl_image_info
*spl_image,
mhdr->blockid);
return -EINVAL;
}
-#endif
spl_image->offset = mhdr->srcaddr;
-#ifdef CONFIG_SPL_SATA
/*
* For SATA srcaddr is specified in number of sectors.
* The main header is must be stored at sector number 1.
* This expects that sector size is 512 bytes and recalculates
* data offset to bytes relative to the main header.
*/
- if (mhdr->blockid == IBR_HDR_SATA_ID) {
+ if (IS_ENABLED(CONFIG_SPL_SATA) && mhdr->blockid == IBR_HDR_SATA_ID) {
if (spl_image->offset < 1) {
printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",
spl_image->offset);
@@ -197,17 +193,14 @@ int spl_parse_board_header(struct spl_image_info
*spl_image,
spl_image->offset -= 1;
spl_image->offset *= 512;
}
-#endif
-#ifdef CONFIG_SPL_MMC
/*
* For SDIO (eMMC) srcaddr is specified in number of sectors.
* This expects that sector size is 512 bytes and recalculates
* data offset to bytes.
*/
- if (mhdr->blockid == IBR_HDR_SDIO_ID)
+ if (IS_ENABLED(CONFIG_SPL_MMC) && mhdr->blockid == IBR_HDR_SDIO_ID)
spl_image->offset *= 512;
-#endif
if (spl_image->offset % 4 != 0) {
printf("ERROR: Wrong srcaddr (%u) in kwbimage\n",
@@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
timer_init();
/* Armada 375 does not support SerDes and DDR3 init yet */
-#if !defined(CONFIG_ARMADA_375)
- /* First init the serdes PHY's */
- serdes_phy_config();
-
- /* Setup DDR */
- ret = ddr3_init();
- if (ret) {
- debug("ddr3_init() failed: %d\n", ret);
- hang();
+ if (!IS_ENABLED(CONFIG_ARMADA_375)) {
+ /* First init the serdes PHY's */
+ serdes_phy_config();
+
+ /* Setup DDR */
+ ret = ddr3_init();
+ if (ret) {
+ debug("ddr3_init() failed: %d\n", ret);
+ hang();
+ }
}
-#endif
/* Initialize Auto Voltage Scaling */
mv_avs_init();
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
[PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
From: Marek Behún
Use the preferred
if (IS_ENABLED(X))
instead of
#ifdef X
where possible.
There are still places where this is not possible or is more complicated
to convert in this file. Leave those be for now.
Signed-off-by: Marek Behún
---
arch/arm/mach-mvebu/spl.c | 43 ---
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 97d7aea179..7dbe8eeba3 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -150,26 +150,24 @@ int spl_parse_board_header(struct spl_image_info
*spl_image,
return -EINVAL;
}
-#ifdef CONFIG_SPL_SPI_FLASH_SUPPORT
- if (bootdev->boot_device == BOOT_DEVICE_SPI &&
+ if (IS_ENABLED(CONFIG_SPL_SPI_FLASH_SUPPORT) &&
+ bootdev->boot_device == BOOT_DEVICE_SPI &&
mhdr->blockid != IBR_HDR_SPI_ID) {
printf("ERROR: Wrong blockid (%u) in SPI kwbimage\n",
mhdr->blockid);
return -EINVAL;
}
-#endif
-#ifdef CONFIG_SPL_SATA
- if (bootdev->boot_device == BOOT_DEVICE_SATA &&
+ if (IS_ENABLED(CONFIG_SPL_SATA) &&
+ bootdev->boot_device == BOOT_DEVICE_SATA &&
mhdr->blockid != IBR_HDR_SATA_ID) {
printf("ERROR: Wrong blockid (%u) in SATA kwbimage\n",
mhdr->blockid);
return -EINVAL;
}
-#endif
-#ifdef CONFIG_SPL_MMC
- if ((bootdev->boot_device == BOOT_DEVICE_MMC1 ||
+ if (IS_ENABLED(CONFIG_SPL_MMC) &&
+ (bootdev->boot_device == BOOT_DEVICE_MMC1 ||
bootdev->boot_device == BOOT_DEVICE_MMC2 ||
bootdev->boot_device == BOOT_DEVICE_MMC2_2) &&
mhdr->blockid != IBR_HDR_SDIO_ID) {
@@ -177,18 +175,16 @@ int spl_parse_board_header(struct spl_image_info
*spl_image,
mhdr->blockid);
return -EINVAL;
}
-#endif
spl_image->offset = mhdr->srcaddr;
-#ifdef CONFIG_SPL_SATA
/*
* For SATA srcaddr is specified in number of sectors.
* The main header is must be stored at sector number 1.
* This expects that sector size is 512 bytes and recalculates
* data offset to bytes relative to the main header.
*/
- if (mhdr->blockid == IBR_HDR_SATA_ID) {
+ if (IS_ENABLED(CONFIG_SPL_SATA) && mhdr->blockid == IBR_HDR_SATA_ID) {
if (spl_image->offset < 1) {
printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",
spl_image->offset);
@@ -197,17 +193,14 @@ int spl_parse_board_header(struct spl_image_info
*spl_image,
spl_image->offset -= 1;
spl_image->offset *= 512;
}
-#endif
-#ifdef CONFIG_SPL_MMC
/*
* For SDIO (eMMC) srcaddr is specified in number of sectors.
* This expects that sector size is 512 bytes and recalculates
* data offset to bytes.
*/
- if (mhdr->blockid == IBR_HDR_SDIO_ID)
+ if (IS_ENABLED(CONFIG_SPL_MMC) && mhdr->blockid == IBR_HDR_SDIO_ID)
spl_image->offset *= 512;
-#endif
if (spl_image->offset % 4 != 0) {
printf("ERROR: Wrong srcaddr (%u) in kwbimage\n",
@@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
timer_init();
/* Armada 375 does not support SerDes and DDR3 init yet */
-#if !defined(CONFIG_ARMADA_375)
- /* First init the serdes PHY's */
- serdes_phy_config();
-
- /* Setup DDR */
- ret = ddr3_init();
- if (ret) {
- debug("ddr3_init() failed: %d\n", ret);
- hang();
+ if (!IS_ENABLED(CONFIG_ARMADA_375)) {
+ /* First init the serdes PHY's */
+ serdes_phy_config();
+
+ /* Setup DDR */
+ ret = ddr3_init();
+ if (ret) {
+ debug("ddr3_init() failed: %d\n", ret);
+ hang();
+ }
}
-#endif
/* Initialize Auto Voltage Scaling */
mv_avs_init();
--
2.32.0
