From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Wed, 13 Aug 2014 15:41:32 -0500 Subject: [U-Boot] [PATCH 3/4] mtd: nand: add Freescale NFC driver In-Reply-To: <87wqadl3h6.fsf@nbsps.com> References: <4ea124ab817db6e3dee2067aadd6db14643990f5.1407312577.git.stefan@agner.ch> <1407796426.7427.100.camel@snotra.buserror.net> <1380dbe3110cbf9359220a78d69e1896@agner.ch> <1407881820.7427.137.camel@snotra.buserror.net> <87wqadl3h6.fsf@nbsps.com> Message-ID: <1407962492.7427.168.camel@snotra.buserror.net> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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