From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-bk0-x22c.google.com ([2a00:1450:4008:c01::22c]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VJOHi-0005Nv-Uz for linux-mtd@lists.infradead.org; Tue, 10 Sep 2013 13:47:18 +0000 Received: by mail-bk0-f44.google.com with SMTP id mz10so3000051bkb.3 for ; Tue, 10 Sep 2013 06:46:52 -0700 (PDT) Message-ID: <522F22C9.1050305@gmail.com> Date: Tue, 10 Sep 2013 15:46:49 +0200 From: Daniel Mack MIME-Version: 1.0 To: Ezequiel Garcia Subject: Re: [RFC/PATCH v2] mtd: nand: pxa3xx: Remove redundant device probing References: <1378811821-14766-1-git-send-email-ezequiel.garcia@free-electrons.com> <1378811821-14766-2-git-send-email-ezequiel.garcia@free-electrons.com> In-Reply-To: <1378811821-14766-2-git-send-email-ezequiel.garcia@free-electrons.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Thomas Petazzoni , Lior Amsalem , Tawfik Bayouk , haojian.zhuang@gmail.com, b32955@freescale.com, linux-mtd@lists.infradead.org, Gregory Clement , Brian Norris , Willy Tarreau List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 10.09.2013 13:17, Ezequiel Garcia wrote: > There's no need to go through this internal probe/auto-detect > procedure, since the nand core code will take care of that. > This commit removes the configuration and detection functions, > together with the built-in flash device table. > > Besides being unneeded, it's also wrong to take care of such details > wich rightfully belong to the NAND base code. Removing this wrong > code, prevents the proliferation of the same mistake in future drivers. > > This commit has the effect of forcing the "keep_config" option. I get the following build warning with this patch: drivers/mtd/nand/pxa3xx_nand.c:221:13: warning: ‘pxa3xx_nand_set_timing’ defined but not used [-Wunused-function] Apart from that, this seems to work fine on my board, but I suspect that it would break systems where the NAND controller is not initialized from the bootloader, right? Thanks, Daniel > > Signed-off-by: Ezequiel Garcia > --- > drivers/mtd/nand/pxa3xx_nand.c | 138 ++--------------------------------------- > 1 file changed, 4 insertions(+), 134 deletions(-) > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > index dd03dfd..bfcc588 100644 > --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -212,28 +212,6 @@ static bool use_dma = 1; > module_param(use_dma, bool, 0444); > MODULE_PARM_DESC(use_dma, "enable DMA for data transferring to/from NAND HW"); > > -static struct pxa3xx_nand_timing timing[] = { > - { 40, 80, 60, 100, 80, 100, 90000, 400, 40, }, > - { 10, 0, 20, 40, 30, 40, 11123, 110, 10, }, > - { 10, 25, 15, 25, 15, 30, 25000, 60, 10, }, > - { 10, 35, 15, 25, 15, 25, 25000, 60, 10, }, > -}; > - > -static struct pxa3xx_nand_flash builtin_flash_types[] = { > -{ "DEFAULT FLASH", 0, 0, 2048, 8, 8, 0, &timing[0] }, > -{ "64MiB 16-bit", 0x46ec, 32, 512, 16, 16, 4096, &timing[1] }, > -{ "256MiB 8-bit", 0xdaec, 64, 2048, 8, 8, 2048, &timing[1] }, > -{ "4GiB 8-bit", 0xd7ec, 128, 4096, 8, 8, 8192, &timing[1] }, > -{ "128MiB 8-bit", 0xa12c, 64, 2048, 8, 8, 1024, &timing[2] }, > -{ "128MiB 16-bit", 0xb12c, 64, 2048, 16, 16, 1024, &timing[2] }, > -{ "512MiB 8-bit", 0xdc2c, 64, 2048, 8, 8, 4096, &timing[2] }, > -{ "512MiB 16-bit", 0xcc2c, 64, 2048, 16, 16, 4096, &timing[2] }, > -{ "256MiB 16-bit", 0xba20, 64, 2048, 16, 16, 2048, &timing[3] }, > -}; > - > -/* Define a default flash type setting serve as flash detecting only */ > -#define DEFAULT_FLASH_TYPE (&builtin_flash_types[0]) > - > #define NDTR0_tCH(c) (min((c), 7) << 19) > #define NDTR0_tCS(c) (min((c), 7) << 16) > #define NDTR0_tWH(c) (min((c), 7) << 11) > @@ -843,53 +821,7 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this) > return 0; > } > > -static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info, > - const struct pxa3xx_nand_flash *f) > -{ > - struct platform_device *pdev = info->pdev; > - struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev); > - struct pxa3xx_nand_host *host = info->host[info->cs]; > - uint32_t ndcr = 0x0; /* enable all interrupts */ > - > - if (f->page_size != 2048 && f->page_size != 512) { > - dev_err(&pdev->dev, "Current only support 2048 and 512 size\n"); > - return -EINVAL; > - } > - > - if (f->flash_width != 16 && f->flash_width != 8) { > - dev_err(&pdev->dev, "Only support 8bit and 16 bit!\n"); > - return -EINVAL; > - } > - > - /* calculate flash information */ > - host->page_size = f->page_size; > - host->read_id_bytes = (f->page_size == 2048) ? 4 : 2; > - > - /* calculate addressing information */ > - host->col_addr_cycles = (f->page_size == 2048) ? 2 : 1; > - > - if (f->num_blocks * f->page_per_block > 65536) > - host->row_addr_cycles = 3; > - else > - host->row_addr_cycles = 2; > - > - ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0; > - ndcr |= (host->col_addr_cycles == 2) ? NDCR_RA_START : 0; > - ndcr |= (f->page_per_block == 64) ? NDCR_PG_PER_BLK : 0; > - ndcr |= (f->page_size == 2048) ? NDCR_PAGE_SZ : 0; > - ndcr |= (f->flash_width == 16) ? NDCR_DWIDTH_M : 0; > - ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0; > - > - ndcr |= NDCR_RD_ID_CNT(host->read_id_bytes); > - ndcr |= NDCR_SPARE_EN; /* enable spare by default */ > - > - info->reg_ndcr = ndcr; > - > - pxa3xx_nand_set_timing(host, f->timing); > - return 0; > -} > - > -static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info) > +static void pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info) > { > /* > * We set 0 by hard coding here, for we don't support keep_config > @@ -909,7 +841,6 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info) > info->reg_ndcr = ndcr & ~NDCR_INT_MASK; > info->ndtr0cs0 = nand_readl(info, NDTR0CS0); > info->ndtr1cs0 = nand_readl(info, NDTR1CS0); > - return 0; > } > > /* the maximum possible buffer size for large page with OOB data > @@ -982,17 +913,10 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info) > static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info) > { > struct mtd_info *mtd; > - int ret; > mtd = info->host[info->cs]->mtd; > - /* use the common timing to make a try */ > - ret = pxa3xx_nand_config_flash(info, &builtin_flash_types[0]); > - if (ret) > - return ret; > - > pxa3xx_nand_cmdfunc(mtd, NAND_CMD_RESET, 0, 0); > if (info->is_ready) > return 0; > - > return -ENODEV; > } > > @@ -1000,72 +924,18 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd) > { > struct pxa3xx_nand_host *host = mtd->priv; > struct pxa3xx_nand_info *info = host->info_data; > - struct platform_device *pdev = info->pdev; > - struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev); > - struct nand_flash_dev pxa3xx_flash_ids[2], *def = NULL; > - const struct pxa3xx_nand_flash *f = NULL; > struct nand_chip *chip = mtd->priv; > - uint32_t id = -1; > - uint64_t chipsize; > - int i, ret, num; > + int ret; > > - if (pdata->keep_config && !pxa3xx_nand_detect_config(info)) > - goto KEEP_CONFIG; > + pxa3xx_nand_detect_config(info); > > ret = pxa3xx_nand_sensing(info); > if (ret) { > dev_info(&info->pdev->dev, "There is no chip on cs %d!\n", > info->cs); > - > - return ret; > - } > - > - chip->cmdfunc(mtd, NAND_CMD_READID, 0, 0); > - id = *((uint16_t *)(info->data_buff)); > - if (id != 0) > - dev_info(&info->pdev->dev, "Detect a flash id %x\n", id); > - else { > - dev_warn(&info->pdev->dev, > - "Read out ID 0, potential timing set wrong!!\n"); > - > - return -EINVAL; > - } > - > - num = ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 1; > - for (i = 0; i < num; i++) { > - if (i < pdata->num_flash) > - f = pdata->flash + i; > - else > - f = &builtin_flash_types[i - pdata->num_flash + 1]; > - > - /* find the chip in default list */ > - if (f->chip_id == id) > - break; > - } > - > - if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 1)) { > - dev_err(&info->pdev->dev, "ERROR!! flash not defined!!!\n"); > - > - return -EINVAL; > - } > - > - ret = pxa3xx_nand_config_flash(info, f); > - if (ret) { > - dev_err(&info->pdev->dev, "ERROR! Configure failed\n"); > return ret; > } > > - pxa3xx_flash_ids[0].name = f->name; > - pxa3xx_flash_ids[0].dev_id = (f->chip_id >> 8) & 0xffff; > - pxa3xx_flash_ids[0].pagesize = f->page_size; > - chipsize = (uint64_t)f->num_blocks * f->page_per_block * f->page_size; > - pxa3xx_flash_ids[0].chipsize = chipsize >> 20; > - pxa3xx_flash_ids[0].erasesize = f->page_size * f->page_per_block; > - if (f->flash_width == 16) > - pxa3xx_flash_ids[0].options = NAND_BUSWIDTH_16; > - pxa3xx_flash_ids[1].name = NULL; > - def = pxa3xx_flash_ids; > -KEEP_CONFIG: > chip->ecc.mode = NAND_ECC_HW; > chip->ecc.size = host->page_size; > chip->ecc.strength = 1; > @@ -1073,7 +943,7 @@ KEEP_CONFIG: > if (info->reg_ndcr & NDCR_DWIDTH_M) > chip->options |= NAND_BUSWIDTH_16; > > - if (nand_scan_ident(mtd, 1, def)) > + if (nand_scan_ident(mtd, 1, NULL)) > return -ENODEV; > /* calculate addressing information */ > if (mtd->writesize >= 2048) >