From: Ben Dooks <ben-mtd@fluff.org>
To: Jonathan McDowell <noodles@earth.li>
Cc: linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
Ben Dooks <ben-mtd@fluff.org>
Subject: Re: [PATCH] Make nand block functions use provided byte/word helpers.
Date: Wed, 1 Mar 2006 11:50:20 +0000 [thread overview]
Message-ID: <20060301115020.GD25880@home.fluff.org> (raw)
In-Reply-To: <20060228222559.GC14749@earth.li>
On Tue, Feb 28, 2006 at 10:25:59PM +0000, Jonathan McDowell wrote:
> On Tue, Feb 28, 2006 at 10:12:43PM +0000, Ben Dooks wrote:
> > On Tue, Feb 28, 2006 at 08:59:03PM +0000, Jonathan McDowell wrote:
> > > I've been writing a NAND driver for the flash on the Amstrad E3. One of
> > > the peculiarities of this device is that the write & read enable lines
> > > are on a latch, rather than strobed by the act of reading/writing from
> > > the data latch. As such I've got custom read_byte/write_byte functions
> > > defined. However the nand_*_buf functions in drivers/mtd/nand/nand_base.c
> > > are all appropriate, except for the fact they call readb/writeb
> > > themselves, instead of using this->read_byte or this->write_byte. The
> > > patch below changes them to use these functions, meaning a driver just
> > > needs to define read_byte and write_byte functions and gains all the
> > > nand_*_buf functions free.
> >
> > Why not make life easier on everyone else by over-riding the
> > functions for read/write buffer (etc) in the nand driver... less
> > intrusive into the core code!
>
> If the patch is deemed too intrusive then that's what I'll do; I nearly
> did so originally but when I caught myself cut and pasting the code from
> nand_base I thought my patch was the cleaner way.
>
> The patch shouldn't cause any extra work for anyone that I can see and I
> don't think it's intrusive at all; I submitted it because I figured it
> might save someone else some work down the line as well.
Well, a small iritation is that it adds work for the read and
write byte function, as a function call adds instructions to
the path. Also, do you want to check that it dosen't break
any existing drivers?
Secondly, you'll probably have a better piece of code for the
E3 driver if you did the block calls with only one setup for
the r/w enable lines per block, instead of checking them for
every byte done. You may even want to use readsb/writesb or
DMA to accelerate the operation.
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
WARNING: multiple messages have this Message-ID (diff)
From: Ben Dooks <ben-mtd@fluff.org>
To: Jonathan McDowell <noodles@earth.li>
Cc: Ben Dooks <ben-mtd@fluff.org>,
linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Make nand block functions use provided byte/word helpers.
Date: Wed, 1 Mar 2006 11:50:20 +0000 [thread overview]
Message-ID: <20060301115020.GD25880@home.fluff.org> (raw)
In-Reply-To: <20060228222559.GC14749@earth.li>
On Tue, Feb 28, 2006 at 10:25:59PM +0000, Jonathan McDowell wrote:
> On Tue, Feb 28, 2006 at 10:12:43PM +0000, Ben Dooks wrote:
> > On Tue, Feb 28, 2006 at 08:59:03PM +0000, Jonathan McDowell wrote:
> > > I've been writing a NAND driver for the flash on the Amstrad E3. One of
> > > the peculiarities of this device is that the write & read enable lines
> > > are on a latch, rather than strobed by the act of reading/writing from
> > > the data latch. As such I've got custom read_byte/write_byte functions
> > > defined. However the nand_*_buf functions in drivers/mtd/nand/nand_base.c
> > > are all appropriate, except for the fact they call readb/writeb
> > > themselves, instead of using this->read_byte or this->write_byte. The
> > > patch below changes them to use these functions, meaning a driver just
> > > needs to define read_byte and write_byte functions and gains all the
> > > nand_*_buf functions free.
> >
> > Why not make life easier on everyone else by over-riding the
> > functions for read/write buffer (etc) in the nand driver... less
> > intrusive into the core code!
>
> If the patch is deemed too intrusive then that's what I'll do; I nearly
> did so originally but when I caught myself cut and pasting the code from
> nand_base I thought my patch was the cleaner way.
>
> The patch shouldn't cause any extra work for anyone that I can see and I
> don't think it's intrusive at all; I submitted it because I figured it
> might save someone else some work down the line as well.
Well, a small iritation is that it adds work for the read and
write byte function, as a function call adds instructions to
the path. Also, do you want to check that it dosen't break
any existing drivers?
Secondly, you'll probably have a better piece of code for the
E3 driver if you did the block calls with only one setup for
the r/w enable lines per block, instead of checking them for
every byte done. You may even want to use readsb/writesb or
DMA to accelerate the operation.
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
next prev parent reply other threads:[~2006-03-01 11:50 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-02-28 20:59 [PATCH] Make nand block functions use provided byte/word helpers Jonathan McDowell
2006-02-28 20:59 ` Jonathan McDowell
2006-02-28 22:12 ` Ben Dooks
2006-02-28 22:25 ` Jonathan McDowell
2006-03-01 11:50 ` Ben Dooks [this message]
2006-03-01 11:50 ` Ben Dooks
2006-03-01 12:29 ` Jonathan McDowell
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=20060301115020.GD25880@home.fluff.org \
--to=ben-mtd@fluff.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=noodles@earth.li \
/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.