All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Marek Behún" <marek.behun@nic.cz>
To: "Pali Rohár" <pali@kernel.org>
Cc: Stefan Roese <sr@denx.de>, u-boot@lists.denx.de
Subject: Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
Date: Thu, 16 Dec 2021 23:17:36 +0100	[thread overview]
Message-ID: <20211216231736.77180c27@thinkpad> (raw)
In-Reply-To: <20211216230903.2e8e37f7@thinkpad>

On Thu, 16 Dec 2021 23:09:03 +0100
Marek Behún <kabel@kernel.org> wrote:

> On Thu, 16 Dec 2021 19:16:40 +0100
> Pali Rohár <pali@kernel.org> 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

  reply	other threads:[~2021-12-16 22:17 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-26 14:37 [PATCH u-boot-marvell v2 0/9] More verifications for kwbimage in SPL Marek Behún
2021-11-26 14:37 ` [PATCH u-boot-marvell v2 1/9] arm: mvebu: Check that kwbimage offset and blocksize are valid Marek Behún
2021-11-26 14:37 ` [PATCH u-boot-marvell v2 2/9] SPL: Add struct spl_boot_device parameter into spl_parse_board_header() Marek Behún
2021-11-26 14:37 ` [PATCH u-boot-marvell v2 3/9] arm: mvebu: Check that kwbimage blockid matches boot mode Marek Behún
2021-11-26 14:37 ` [PATCH u-boot-marvell v2 4/9] SPL: Add support for checking board / BootROM specific image types Marek Behún
2021-11-26 14:37 ` [PATCH u-boot-marvell v2 5/9] arm: mvebu: Check for kwbimage data checksum Marek Behún
2021-11-26 14:37 ` [PATCH u-boot-marvell v2 6/9] arm: mvebu: spl: Print srcaddr in error message Marek Behún
2021-11-30  6:20   ` Stefan Roese
2021-12-14 11:10   ` Pali Rohár
2021-12-14 13:11     ` Marek Behún
2021-11-26 14:37 ` [PATCH u-boot-marvell v2 7/9] arm: mvebu: spl: Use preferred types u8/u16/u32 instead of uintN_t Marek Behún
2021-11-30  6:20   ` Stefan Roese
2021-11-26 14:37 ` [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible Marek Behún
2021-11-30  6:22   ` Stefan Roese
2021-12-14  9:36   ` Pali Rohár
2021-12-14  9:45     ` Marek Behún
2021-12-14 11:12       ` Pali Rohár
2021-12-14 12:45         ` Marek Behún
2021-12-14 12:48           ` Stefan Roese
2021-12-14 13:01             ` Marek Behún
2021-12-16 18:16               ` Pali Rohár
2021-12-16 22:09                 ` Marek Behún
2021-12-16 22:17                   ` Marek Behún [this message]
2021-11-26 14:37 ` [PATCH u-boot-marvell v2 9/9] arm: mvebu: spl: Fix 100 column exceeds Marek Behún
2021-11-30  6:22   ` Stefan Roese

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211216231736.77180c27@thinkpad \
    --to=marek.behun@nic.cz \
    --cc=pali@kernel.org \
    --cc=sr@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.