public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Roland Dreier <rdreier@cisco.com>
To: "Bryan O'Sullivan" <bos@pathscale.com>
Cc: linux-kernel@vger.kernel.org, mpm@selenic.com, akpm@osdl.org,
	hch@infradead.org
Subject: Re: [PATCH 1 of 3] Introduce __memcpy_toio32
Date: Tue, 27 Dec 2005 17:10:14 -0800	[thread overview]
Message-ID: <adazmmmc9hl.fsf@cisco.com> (raw)
In-Reply-To: <7b7b442a4d6338ae8ca7.1135726915@eng-12.pathscale.com> (Bryan O'Sullivan's message of "Tue, 27 Dec 2005 15:41:55 -0800")

A couple of comments here:

 > +/*
 > + * Copy data to an MMIO region.  MMIO space accesses are performed
 > + * in the sizes indicated in each function's name.
 > + */
 > +void fastcall __memcpy_toio32(volatile void __iomem *d, const void *s, size_t count)
 > +{
 > +	volatile u32 __iomem *dst = d;
 > +	const u32 *src = s;
 > +
 > +	while (--count >= 0) {
 > +		__raw_writel(*src++, dst++);
 > +	}

I think the principle of least surprise calls for memcpy_toio32 to be
ordered the same way memcpy_toio is.  In other words there should be a
wmb() after the loop.

Also, no need for the { } for the while loop.

 > +}
 > +
 > +EXPORT_SYMBOL_GPL(__memcpy_toio32);

You're adding this symbol and exporting it even if the arch will
supply its own version.  So this is pure kernel .text bloat...

 - R.

  reply	other threads:[~2005-12-28  1:10 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-12-27 23:41 [PATCH 0 of 3] Add memcpy_toio32, a 32-bit MMIO copy routine Bryan O'Sullivan
2005-12-27 23:41 ` [PATCH 1 of 3] Introduce __memcpy_toio32 Bryan O'Sullivan
2005-12-28  1:10   ` Roland Dreier [this message]
2005-12-28 14:40     ` Bryan O'Sullivan
2005-12-28 14:51       ` Matt Mackall
2005-12-30 23:46         ` Adrian Bunk
2005-12-30 23:44           ` Matt Mackall
2005-12-31  0:23             ` Linus Torvalds
2005-12-31  0:31               ` (OT) " Jan Engelhardt
2005-12-31  0:44                 ` Linus Torvalds
2005-12-31 21:24               ` Adrian Bunk
2005-12-28 19:23       ` Roland Dreier
2005-12-28  1:11   ` Roland Dreier
2005-12-28  4:07     ` Matt Mackall
2005-12-28  3:52   ` Matt Mackall
2005-12-28 14:47     ` Bryan O'Sullivan
2005-12-28 14:55       ` Matt Mackall
2005-12-28 15:18     ` Geert Uytterhoeven
2005-12-28 15:52       ` Bryan O'Sullivan
2005-12-27 23:41 ` [PATCH 2 of 3] memcpy32 for x86_64 Bryan O'Sullivan
2005-12-28  4:22   ` Matt Mackall
2005-12-28  7:54     ` Denis Vlasenko
2005-12-28 14:52     ` Bryan O'Sullivan
2006-01-06  9:12   ` Pavel Machek
2006-01-06 16:02     ` Bryan O'Sullivan
2005-12-27 23:41 ` [PATCH 3 of 3] Add memcpy_toio32 to each arch Bryan O'Sullivan
     [not found] <200512280603.jBS63WGB031117@taverner.CS.Berkeley.EDU>
2005-12-28  6:20 ` [PATCH 1 of 3] Introduce __memcpy_toio32 Matt Mackall

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=adazmmmc9hl.fsf@cisco.com \
    --to=rdreier@cisco.com \
    --cc=akpm@osdl.org \
    --cc=bos@pathscale.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpm@selenic.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