Re[2]: [PATCH 3/3] mtd: nand_base: Use readsb/readsw/writesb/writesw if possible

[Alexander Shiyan]

> 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 at 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 at 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.

---