All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bill Pringlemeir <bpringlemeir@nbsps.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] mtd: vf610_nfc: mark page as dirty on block erase
Date: Mon, 30 Mar 2015 17:35:27 -0400	[thread overview]
Message-ID: <87bnjanp7k.fsf@nbsps.com> (raw)
In-Reply-To: <1427748489.22867.187.camel@freescale.com> (Scott Wood's message of "Mon, 30 Mar 2015 15:48:09 -0500")

On 30 Mar 2015, scottwood at freescale.com wrote:

> On Mon, 2015-03-30 at 22:40 +0200, Stefan Agner wrote:

>> However, if removing the caching in the driver would lead to a
>> performance drop, I would rather prefer to keep it...

> What is special about this controller, that caching makes sense here
> but not on other controllers?  If it makes sense everywhere, then the
> upper layer is the place to do it.

While I observed some performance differences, but that is an aside.  I
think the mxc driver is similar in sub-page (or partial page) support.
Before doing a sub-page write, it reads a full page and then copies the
sub-page update to the buffer to be reprogrammed.  This works fine if
each and every sub-page has separate OOB data for software ECC.  The
hardware ECC of this vf610_nfc controller doesn't support this.

At least that is my understanding of this [mxc_nand] code,

        case NAND_CMD_SEQIN:
                if (column >= mtd->writesize) {
                        /*
                         * before sending SEQIN command for partial write,
                         * we need read one page out. FSL NFC does not support
                         * partial write. It always sends out 512+ecc+512+ecc
                         * for large page nand flash. But for small page nand
                         * flash, it does support SPARE ONLY operation.
                         */
                        if (host->pagesize_2k) {
                                /* call ourself to read a page */
                                mxc_nand_command(mtd, NAND_CMD_READ0, 0,
                                                page_addr);
                        }

in 'drivers/mtd/nand/mxc_nand.c'.  So, originally this was mainly for
sub-pages (an optimization but also functionality).

So the 'caching' is just to preserve read data before a partial write as
the vf610 requires a full page (like the mxc).  The same 'caching' was
keep for 'write followed by read' when the ECC is successful.

I realize that the upper layers may not like this so I mentioned it; I
guess that is proof that the patch is getting a good review and I am not
causing problems for Stefan ;)

I also agree with Stefan that setting the SECSZ in the 2nd patch will
result in less data transfer (and maybe less current/power and system
noise).  However, this will be inside the NFC controller.  Talking over
the AHB to the NFC controller is quite slow on the Vybrid.  Certainly it
seems a little more elegant to tell the controller to transfer nothing
(and that we [programmers] expect nothing as a sort of documentation)?

I think it would be worthwhile to benchmark without the cache.  Or maybe
Stefan already has some numbers?  Upper layers doing partial pages will
definitely benefit with the 'cache'; we would also need more DDR memory
as the NFC controller memory is being used as a scratch buffer.

Fwiw,
Bill Pringlemeir.

      parent reply	other threads:[~2015-03-30 21:35 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-24 16:54 [U-Boot] [PATCH 1/2] mtd: vf610_nfc: mark page as dirty on block erase Stefan Agner
2015-03-24 16:54 ` [U-Boot] [PATCH 2/2] mtd: vf610_nfc: specify transfer size before each transfer Stefan Agner
2015-03-30 17:02 ` [U-Boot] [PATCH 1/2] mtd: vf610_nfc: mark page as dirty on block erase Bill Pringlemeir
2015-03-30 20:14   ` Stefan Agner
2015-03-30 20:46     ` Scott Wood
2015-03-30 20:34   ` Scott Wood
2015-03-30 20:40     ` Stefan Agner
2015-03-30 20:48       ` Scott Wood
2015-03-30 21:26         ` Stefan Agner
2015-03-30 22:15           ` Scott Wood
2015-03-30 22:24             ` Stefan Agner
2015-03-31  4:34               ` Scott Wood
2015-03-31 15:02                 ` Bill Pringlemeir
2015-04-02 23:48                   ` Scott Wood
2015-04-03 18:09                     ` Stefan Agner
2015-04-03 20:15                       ` Scott Wood
2015-04-03 20:28                         ` Stefan Agner
2015-04-03 21:03                           ` Scott Wood
2015-04-07 14:06                             ` Bill Pringlemeir
2015-04-07 16:02                               ` Scott Wood
2015-04-07 17:54                                 ` Bill Pringlemeir
2015-04-07 21:09                                   ` Scott Wood
2015-03-30 21:35         ` Bill Pringlemeir [this message]

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=87bnjanp7k.fsf@nbsps.com \
    --to=bpringlemeir@nbsps.com \
    --cc=u-boot@lists.denx.de \
    /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.