public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ARM:asm:io.h use static inline
Date: Sat, 5 Jul 2014 15:21:18 +0200	[thread overview]
Message-ID: <E1X3Pu3-0006IN-Ry@janus> (raw)
In-Reply-To: <1404560207.2417.26.camel@yellow>

Hi Jeroen,

(sorry for the near-duplicate, and see question at end)

On Sat, 05 Jul 2014 13:36:47 +0200, Jeroen Hofstee
<jeroen@myspectrum.nl> wrote:

> Hello Albert,
> 
> On za, 2014-07-05 at 11:13 +0200, Albert ARIBAUD wrote:
> > Hi Jeroen,
> > 
> > On Sun, 22 Jun 2014 23:10:39 +0200, Jeroen Hofstee
> > <jeroen@myspectrum.nl> wrote:
> > 
> > > When compiling u-boot with W=1 the extern inline void for
> > > read* is likely causing the most noise. gcc / clang will
> > > warn there is never a actual declaration for these functions.
> > > Instead of declaring these extern make them static inline so
> > > it is actually declared.
> > > 
> > > cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> > > Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
> > > ---
> > 
> > Ok, so the obvious question: what makes you decide to switch to 'static
> > inline' rather than provide the extern versions that 'extern static'
> > calls for?
> 
> Assuming your question is, why didn't you just add the prototypes instead?

It was more along the lines of "were you aware that you had a choice
there?"

> Well if we wanted to be brave gnu99 citizens we should provide the
> prototypes, the extern inline version and a separated definition in
> case the compiler fails / is not in the mood to inline the function.
> This quite fragile / some housekeeping. Furthermore it is gnu specific
> and likely fails with gcc -std=c99 as well.
> 
> Making them static inline there is always a single definition and it
> is up to the compiler to either inline it or make it a static function
> by it self. Since we were already relying on the compiler to inline
> it (at least I am unaware that there are non inline version around),
> this boils down to the same thing, but without warnings.
> 
> And... I can likely drop this one as well[1], although I haven't
> checked yet. linux does the same btw for __LINUX_ARM_ARCH__ >= 6.
> 
> I can check binary size if that something you wonder about...

As I said, I have no stance on whether 'static inline' or 'extern
inline' was better / more appropriate / other (please specify). I just
wanted to make sure that you had considered both possibilities before
choosing one of them. I am now assured that you have, so all is fine.

> Regards,
> Jeroen
> 
> [1] https://github.com/jhofstee/u-boot/commit/5cd261fecc5397bf5abef82f6a781d8b04992654

Which board do you get warnings for?

Amicalement,
-- 
Albert.

  reply	other threads:[~2014-07-05 13:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-22 21:10 [U-Boot] [PATCH] ARM:asm:io.h use static inline Jeroen Hofstee
2014-07-05  9:13 ` Albert ARIBAUD
2014-07-05 11:36   ` Jeroen Hofstee
2014-07-05 13:21     ` Albert ARIBAUD [this message]
2014-07-05 13:30       ` Jeroen Hofstee
2014-07-05 14:58         ` Albert ARIBAUD
2014-08-30 15:13 ` [U-Boot] " Tom Rini

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=E1X3Pu3-0006IN-Ry@janus \
    --to=albert.u.boot@aribaud.net \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox