From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Sat, 5 Jul 2014 16:58:20 +0200 Subject: [U-Boot] [PATCH] ARM:asm:io.h use static inline In-Reply-To: <53B7FE0E.5090306@myspectrum.nl> References: <1403471439-23370-1-git-send-email-jeroen@myspectrum.nl> <1404560207.2417.26.camel@yellow> <53B7FE0E.5090306@myspectrum.nl> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Jeroen, On Sat, 05 Jul 2014 15:30:54 +0200, Jeroen Hofstee wrote: > Hello Albert, > > On 05-07-14 15:21, Albert ARIBAUD wrote: > > Hi Jeroen, > > > > (sorry for the near-duplicate, and see question at end) > > > > On Sat, 05 Jul 2014 13:36:47 +0200, Jeroen Hofstee > > 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 > >>> 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 > >>>> Signed-off-by: Jeroen Hofstee > >>>> --- > >>> 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?" > yes I was aware of that. > >> 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. > ok > >> Regards, > >> Jeroen > >> > >> [1] https://github.com/jhofstee/u-boot/commit/5cd261fecc5397bf5abef82f6a781d8b04992654 > > Which board do you get warnings for? > Basically any, but as the commit message says, it is just for removing > warnings when building with `make W=1` or clang (which actually errors, > without the other patch). There is no hurry in applying it. But e.g. > for the twister I am sure you will see them, with the additional warnings > enabled. Ok, seeing that it is not needed for customary builds, I'll defer this until after 2014.07. > Regards, > Jeroen Amicalement, -- Albert.