On 01/13/2012 04:10 PM, Simon Glass wrote:
From: Jim Lin jilin@nvidia.com
A device tree is used to configure the NAND, including memory timings and block/pages sizes.
If this node is not present or is disabled, then NAND will not be initialized.
Signed-off-by: Simon Glass sjg@chromium.org
diff --git a/drivers/mtd/nand/tegra2_nand.c b/drivers/mtd/nand/tegra2_nand.c
+/**
- Wait for command completion
- @param reg nand_ctlr structure
- @return
1 - Command completed
0 - Timeout- */
+static int nand_waitfor_cmd_completion(struct nand_ctlr *reg) +{
int i;u32 reg_val;for (i = 0; i < NAND_CMD_TIMEOUT_MS * 1000; i++) {if ((readl(®->command) & CMD_GO) ||!(readl(®->status) &STATUS_RBSY0) ||!(readl(®->isr) &ISR_IS_CMD_DONE)) {udelay(1);continue;}reg_val = readl(®->dma_mst_ctrl);/** If DMA_MST_CTRL_EN_A_ENABLE or* DMA_MST_CTRL_EN_B_ENABLE is set,* that means DMA engine is running, then we* have to wait until* DMA_MST_CTRL_IS_DMA_DONE* is cleared for DMA transfer completion.*/if (reg_val & (DMA_MST_CTRL_EN_A_ENABLE |DMA_MST_CTRL_EN_B_ENABLE)) {if (reg_val & DMA_MST_CTRL_IS_DMA_DONE)return 1;} elsereturn 1;udelay(1);
To be more consistent with the first if/continue block, wouldn't it be better to recast that last if test and udelay as:
if (reg_val & (DMA_MST_CTRL_EN_A_ENABLE | DMA_MST_CTRL_EN_B_ENABLE)) { if (!(reg_val & DMA_MST_CTRL_IS_DMA_DONE)) { udelay(1); continue; } }
break;
+/**
- [DEFAULT] Send command to NAND device
- @param mtd MTD device structure
- @param command the command to be sent
- @param column the column address for this command, -1 if none
- @param page_addr the page address for this command, -1 if none
- */
+static void nand_command(struct mtd_info *mtd, unsigned int command,
int column, int page_addr)+{
register struct nand_chip *chip = mtd->priv;struct nand_info *info;info = (struct nand_info *) chip->priv;/** Write out the command to the device.*/if (mtd->writesize < 2048) {/** Only command NAND_CMD_RESET or NAND_CMD_READID will come* here before mtd->writesize is initialized, we don't have* any action here because page size of NAND HY27UF084G2B* is 2048 bytes and mtd->writesize will be 2048 after* initialized.*/
What if the NAND flash doesn't have a page size of 2048 bytes? The driver shouldn't make such assumptions about the flash chip that happens to be connected. Should the if above be:
if (mtd->writesize == 0)
to be generic?
Should this if branch validate that the command being executed is a legitimate command for the not-yet-fully-initialized case?
+/**
- Set up NAND bus width and page size
- @param info nand_info structure
- @param *reg_val address of reg_val
- @return none - value is set in reg_val
- */
+static void set_bus_width_page_size(struct fdt_nand *config,
u32 *reg_val)+{
if (config->width == 8)*reg_val = CFG_BUS_WIDTH_8BIT;else
Shouldn't that be else if (config->width == 16)
*reg_val = CFG_BUS_WIDTH_16BIT;
... and there be an else clause that returns an error?
if (config->page_data_bytes == 256)*reg_val |= CFG_PAGE_SIZE_256;else if (config->page_data_bytes == 512)*reg_val |= CFG_PAGE_SIZE_512;else if (config->page_data_bytes == 1024)*reg_val |= CFG_PAGE_SIZE_1024;else if (config->page_data_bytes == 2048)*reg_val |= CFG_PAGE_SIZE_2048;
And similarly, and else clause that returns an error here?