From: "Alexander Shiyan" <shc_work@mail.ru>
To: "Brian Norris" <computersforpeace@gmail.com>
Cc: "David Woodhouse" <dwmw2@infradead.org>,
linux-mtd@lists.infradead.org,
"Artem Bityutskiy" <artem.bityutskiy@linux.intel.com>
Subject: Re[2]: [PATCH 3/3] mtd: nand_base: Use readsb/readsw/writesb/writesw if possible
Date: Tue, 05 Mar 2013 11:54:42 +0400 [thread overview]
Message-ID: <1362470082.387553774@f47.mail.ru> (raw)
In-Reply-To: <51359FCE.4060605@gmail.com>
> Hi Alexander,
>
> I'm not an expert on IO accessors, but since you asked...
>
> On 02/28/2013 12:02 AM, Alexander Shiyan wrote:
> > This patch provide using readsb/readsw/writesb/writesw functions
> > if it possible for particular platform.
>
> The commit description does not give enough motivation. Should this
> improve performance? Or is this just a "cleanup"? (If the latter, then
> it fails, as the resulting code is much less clean.)
Basic idea is a performance, because architecture may have a
improved functions for multiple read/write.
>
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > ---
> > drivers/mtd/nand/nand_base.c | 29 ++++++++++++++++++++++++-----
> > 1 files changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index 382b857..4efde01 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -214,11 +214,16 @@ static void nand_select_chip(struct mtd_info *mtd, int chipnr)
> > */
> > static void nand_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> > {
> > - int i;
> > struct nand_chip *chip = mtd->priv;
> >
> > +#ifndef writesb
>
> No one guarantees that 'writesb' will be a macro and not a static inline
> function, do they?
>
> > + int i;
> > +
> > for (i = 0; i < len; i++)
> > writeb(buf[i], chip->IO_ADDR_W);
> > +#else
> > + writesb(chip->IO_ADDR_W, buf, len);
> > +#endif
> > }
>
> These #ifndef's are very ugly and are likely the wrong way(TM). The
> first question I ask when I see this is: why isn't this interface
> available on all platforms? In fact, it seems like it has been dropped
> from the asm-generic headers and specifically recommended against. See:
>
> commit b2656a138ab7bc4e7abd3b1cbd6d1f105c7a7186
> Author: Will Deacon <will.deacon@arm.com>
> Date: Wed Oct 17 16:45:01 2012 +0100
>
> asm-generic: io: remove {read,write} string functions
>
> Quote: "We should encourage driver writers to use the
> io{read,write}{8,16,32}_rep functions instead."
>
> So, an alternative patch might use iowrite8_rep() here unconditionally.
Indeed. So I can remade patch to use ioread/iowrite and remove
unnecessary #ifdefs if you have not additional comments.
---
next prev parent reply other threads:[~2013-03-05 7:54 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-28 8:02 [PATCH 1/3] mtd: nand_base: Removed unnecessary command masking Alexander Shiyan
2013-02-28 8:02 ` [PATCH 2/3] mtd: nand_base: Removed unnecessary cleaning "onfi_version" variable Alexander Shiyan
2013-03-04 5:23 ` Brian Norris
2013-02-28 8:02 ` [PATCH 3/3] mtd: nand_base: Use readsb/readsw/writesb/writesw if possible Alexander Shiyan
2013-03-05 7:33 ` Brian Norris
2013-03-05 7:54 ` Alexander Shiyan [this message]
2013-03-04 20:26 ` [PATCH 1/3] mtd: nand_base: Removed unnecessary command masking Brian Norris
2013-03-05 6:16 ` Re[2]: " Alexander Shiyan
2013-03-11 8:10 ` Artem Bityutskiy
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=1362470082.387553774@f47.mail.ru \
--to=shc_work@mail.ru \
--cc=artem.bityutskiy@linux.intel.com \
--cc=computersforpeace@gmail.com \
--cc=dwmw2@infradead.org \
--cc=linux-mtd@lists.infradead.org \
/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.