linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arm-kernel@lists.infradead.org,
	Thierry Reding <thierry.reding@gmail.com>,
	linux-arch@vger.kernel.org, Russell King <linux@arm.linux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	linux-kernel@vger.kernel.org, Will Deacon <will.deacon@arm.com>
Subject: Re: [PATCH v3 1/3] asm-generic/io.h: Implement generic {read,write}s*()
Date: Sat, 19 Jul 2014 10:53:38 +0200	[thread overview]
Message-ID: <20140719085338.GB31564@ravnborg.org> (raw)
In-Reply-To: <5913515.3g98ODuH6W@wuerfel>

> > A semi related question.
> > Why do we play all these macro tricks in io.h?
> > 
> > Example:
> > 
> >     #define writeb __raw_writeb
> > 
> > The consensus these days is to use static inline to have the
> > correct prototype but this file is contains a lot of these
> > macro conversions.
> > 
> > All these things are not introduced by your patch but now that
> > you show some love and care for this file maybe we could take
> > the next step and bring more order to the current semi chaos?
> 
> The macros are mainly there so we can test their presence with
> #ifdef.

There are two types of macros in io.h:

    #define __raw_writeb __raw_writeb
    #ifndef __raw_writeb

This is the one you talk about which allows an architecture to
provide a local implementation of a function.
These are prefectly OK and are required.


Then there are the other type where one IO access function
may re-use the implementation of another IO access function:

    #ifndef writeb
    #define writeb __raw_writeb
    #endif

This could have been implmented like this:

#ifndef writeb
#define writeb writeb
static inline void writeb(u8 b, volatile void __iomem *addr)
{
   __raw_writeb(b, addr);
}
#endif

In this way the prototype of the function is easy to understand and
we avoid the macro tricks were we blindly replace one function name,
with another function name.
And we also use the same pattarn all over for the various functions.

Concerning the efficiency the compiler should be smart enough to
do the same independent on the two implmentations.

The only drawback I see is that the above is 6 lines of codes,
where the macro expansion is 3 lines of code.

And when we talk about simpler expansion like ioread8()
then we replace one line with 4 lines.

	Sam

  reply	other threads:[~2014-07-19  8:53 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-16 11:01 [PATCH v3 1/3] asm-generic/io.h: Implement generic {read,write}s*() Thierry Reding
2014-07-16 11:01 ` [PATCH v3 2/3] ARM: Use include/asm-generic/io.h Thierry Reding
2014-07-16 11:01   ` Thierry Reding
2014-07-17 12:03   ` Catalin Marinas
2014-07-16 11:01 ` [PATCH v3 3/3] arm64: " Thierry Reding
2014-07-17 12:04   ` Catalin Marinas
2014-07-17 12:26     ` Thierry Reding
2014-07-18 15:50       ` Catalin Marinas
2014-07-17 12:01 ` [PATCH v3 1/3] asm-generic/io.h: Implement generic {read,write}s*() Catalin Marinas
2014-07-18 20:59 ` Sam Ravnborg
2014-07-18 20:59   ` Sam Ravnborg
2014-07-18 21:06   ` Sam Ravnborg
2014-07-18 21:06     ` Sam Ravnborg
2014-07-19  7:38     ` Arnd Bergmann
2014-07-19  8:41       ` Sam Ravnborg
2014-07-19  8:41         ` Sam Ravnborg
2014-07-19  9:05         ` Arnd Bergmann
2014-07-19  9:11           ` Sam Ravnborg
2014-07-19  9:11             ` Sam Ravnborg
2014-07-19 17:21             ` James Bottomley
2014-08-05  9:07             ` Geert Uytterhoeven
2014-08-05  9:14               ` Sam Ravnborg
2014-07-19  7:44   ` Arnd Bergmann
2014-07-19  8:53     ` Sam Ravnborg [this message]
2014-07-19  8:53       ` Sam Ravnborg
2014-07-19  9:10       ` Arnd Bergmann
2014-07-19 12:59   ` [PATCH] asm-generic/io.h: reorder funtions to form logical groups Sam Ravnborg
2014-08-01 14:09     ` Thierry Reding
2014-08-01 14:09       ` Thierry Reding
2014-08-01 22:42       ` Sam Ravnborg

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=20140719085338.GB31564@ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=sboyd@codeaurora.org \
    --cc=thierry.reding@gmail.com \
    --cc=will.deacon@arm.com \
    /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;
as well as URLs for NNTP newsgroup(s).