[U-Boot] [PATCH 3/4] mtd: nand: add Freescale NFC driver

Scott Wood
2014-08-13T20:41:32Z

On Tue, 2014-08-12 at 18:58 -0400, Bill Pringlemeir wrote:
> On 12 Aug 2014, scottwood at freescale.com wrote:
>
> > On Tue, 2014-08-12 at 23:13 +0200, Stefan Agner wrote:
> >> Am 2014-08-12 00:33, schrieb Scott Wood:
> >>> You should always be using raw I/O accessors. If the intent is to
> >>> bypass I/O ordering for certain registers, the raw accessors should
> >>> already be skipping that.
>
> >> Ok, will do.
>
> > Sorry, the above should say "always be using I/O accesors", not always
> > raw ones.
>
> This is probably because of me. There are lines like this for
> configuration,
>
> /* Set configuration register. */
> nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_ADDR_AUTO_INCR_BIT);
> nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_BUFNO_AUTO_INCR_BIT);
> nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT);
> nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT);
> nfc_set(mtd, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT);
>
> If you use some sort of 'volatile', you are saying to the compiler that
> these lines are *not*,
>
> ldr r0, [r1, #CONFIG] # read previous value
> ldr r2, =~MASK
> orr r0, #FAST_FLASH_BIT # set one bit.
> and r0, r2 # clear all bits at once.
> str r0, [r1, #CONFIG] # write it back.
>
> Instead it will change into five different load/modify/stores. The
> memory itself is just like SRAM except a few registers that are actually
> volatile; ie changed via the hardware.

If this is performance-critical then it would be best to modify the code
to explicitly do one read from I/O (if you can't know in advance what
the existing value will be), clear all the bits you're going to clear,
then have one explicit write back to the I/O device -- rather than
treating it as ordinary memory for the compiler to optimize accesses to.

If nothing else, it makes the code clearer to make I/O accesses
explicit, and reduces the likelihood that people see I/O accesses
without accessors and go on to do the same in some other less safe
circumstance.

-Scott