[PATCH v3 1/3] mtd: nand: add asm9260 NFC driver

Oleksij Rempel linux at rempel-privat.de
Thu Jan 1 12:18:44 PST 2015


Am 31.12.2014 um 17:48 schrieb Boris Brezillon:
> Hi Oleksij,
> 
> You should really add a cover letter containing a changelog (updated at
> each new version of your cover letter) so that reviewers can easily
> identify what has changed.
> 
> While you're at it, can you add your mtd test results to the cover
> letter ?

ok.

[snip]

>> +/**
>> + * We can't read less then 32 bits on HW_FIFO_DATA. So, to make
>> + * read_byte and read_word happy, we use sort of cached 32bit read.
>> + * Note: expected values for size should be 1 or 2 bytes.
>> + */
>> +static u32 asm9260_nand_read_cached(struct mtd_info *mtd, int size)
>> +{
>> +	struct asm9260_nand_priv *priv = mtd_to_priv(mtd);
>> +	u8 tmp;
>> +
>> +	if ((priv->read_cache_cnt <= 0) || (priv->read_cache_cnt > 4)) {
> 
>                                             ^ how can this ever happen ?
> 
>> +		asm9260_nand_cmd_comp(mtd, 0);
>> +		priv->read_cache = ioread32(priv->base + HW_FIFO_DATA);
>> +		priv->read_cache_cnt = 4;
>> +	}
>> +
>> +	tmp = priv->read_cache >> (8 * (4 - priv->read_cache_cnt));
>> +	priv->read_cache_cnt -= size;
>> +
>> +	return tmp;
>> +}
>> +
>> +static u8 asm9260_nand_read_byte(struct mtd_info *mtd)
>> +{
>> +	return 0xff & asm9260_nand_read_cached(mtd, 1);
> 
> Maybe this mask operation could be done in asm9260_nand_read_cached, so
> that you won't have to bother in read_byte/read_word functions.

it is same as "return (u8)asm9260_nand_read_cached(mtd, 1);", just makes
sure compiler do not complain.

>> +}
>> +
>> +static u16 asm9260_nand_read_word(struct mtd_info *mtd)
>> +{
>> +	return 0xffff & asm9260_nand_read_cached(mtd, 2);
> 
> You'd better always use read_word, cause if you call read_byte once
> then read_word twice, you'll end up with a wrong value after the second
> read_word (3 bytes consumed, which means there's only 1 remaining byte
> and you're asking for 2).

nand_base use both functions. how can i use only one? For example:
        chip->cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);

        /* Read entire ID string */
        for (i = 0; i < 8; i++)
                id_data[i] = chip->read_byte(mtd);

this wont work with read_word.


>> +}
>> +
>> +static void asm9260_nand_read_buf(struct mtd_info *mtd, u8 *buf, int len)
>> +{
>> +	struct asm9260_nand_priv *priv = mtd_to_priv(mtd);
>> +	dma_addr_t dma_addr;
>> +	int dma_ok = 0;
>> +
>> +	if (len & 0x3) {
>> +		dev_err(priv->dev, "Unsupported length (%x)\n", len);
>> +		return;
>> +	}
>> +
>> +	/*
>> +	 * I hate you UBI for your all vmalloc. Be slow as hell with PIO.
>> +	 * ~ with love from ZeroCopy ~
>> +	 */
>> +	if (!is_vmalloc_addr(buf)) {
>> +		dma_addr = asm9260_nand_dma_set(mtd, buf, DMA_FROM_DEVICE, len);
>> +		dma_ok = !(dma_mapping_error(priv->dev, dma_addr));
>> +	}
>> +	asm9260_nand_cmd_comp(mtd, dma_ok);
>> +
>> +	if (dma_ok) {
>> +		dma_sync_single_for_cpu(priv->dev, dma_addr, len,
>> +				DMA_FROM_DEVICE);
>> +		dma_unmap_single(priv->dev, dma_addr, len, DMA_FROM_DEVICE);
>> +		return;
>> +	}
>> +
>> +	/* fall back to pio mode */
>> +	len >>= 2;
>> +	ioread32_rep(priv->base + HW_FIFO_DATA, buf, len);
> 
> Hm, what if the buf is not aligned on 32bit, or len is not a multiple
> of 4 ?

I can read only buf == page size. All page sizes suported by this
controller are aligned. See "if (len & 0x3) {", and take a look at
asm9260_nand_read_cached - we can read only 32bit.


>> +}
>> +
>> +static void asm9260_nand_write_buf(struct mtd_info *mtd,
>> +		const u8 *buf, int len)
>> +{
>> +	struct asm9260_nand_priv *priv = mtd_to_priv(mtd);
>> +	dma_addr_t dma_addr;
>> +	int dma_ok = 0;
>> +
>> +	if (len & 0x3) {
>> +		dev_err(priv->dev, "Unsupported length (%x)\n", len);
>> +		return;
>> +	}
>> +
>> +	if (!is_vmalloc_addr(buf)) {
>> +		dma_addr = asm9260_nand_dma_set(mtd,
>> +				(void *)buf, DMA_TO_DEVICE, len);
>> +		dma_ok = !(dma_mapping_error(priv->dev, dma_addr));
>> +	}
>> +
>> +	if (dma_ok)
>> +		dma_sync_single_for_device(priv->dev, dma_addr, len,
>> +				DMA_TO_DEVICE);
>> +	asm9260_nand_cmd_comp(mtd, dma_ok);
>> +
>> +	if (dma_ok) {
>> +		dma_unmap_single(priv->dev, dma_addr, len, DMA_TO_DEVICE);
>> +		return;
>> +	}
>> +
>> +	/* fall back to pio mode */
>> +	len >>= 2;
>> +	iowrite32_rep(priv->base + HW_FIFO_DATA, buf, len);
> 
> Ditto
> 
>> +}
>> +
>> +static int asm9260_nand_write_page_raw(struct mtd_info *mtd,
>> +		struct nand_chip *chip, const u8 *buf,
>> +		int oob_required)
>> +{
>> +	struct asm9260_nand_priv *priv = mtd_to_priv(mtd);
>> +
>> +	chip->write_buf(mtd, buf, mtd->writesize);
>> +	if (oob_required)
>> +		chip->ecc.write_oob(mtd, chip, priv->page_cache &
>> +				chip->pagemask);
> 
> Can't you just call
> 		chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> 
> And if it works, then you can just drop this raw function since the
> default one does the exact same thing.

There 3 variants:
- allocate dma == page + oob; and copy each buffer
- use pio and read out oob even if it was not requested
- use dma_map_single, avoid buffer copying, and request oob if needed.

i use last variant even if it meane sending a command to request oob.

> 
>> +	return 0;
>> +}
>> +
>> +static int asm9260_nand_write_page_hwecc(struct mtd_info *mtd,
>> +		struct nand_chip *chip, const u8 *buf,
>> +		int oob_required)
>> +{
>> +	struct asm9260_nand_priv *priv = mtd_to_priv(mtd);
>> +
>> +	asm9260_reg_rmw(priv, HW_CTRL, BM_CTRL_ECC_EN, 0);
>> +	chip->ecc.write_page_raw(mtd, chip, buf, oob_required);
>> +
>> +	return 0;
>> +}
>> +
>> +static unsigned int asm9260_nand_count_ecc(struct asm9260_nand_priv *priv)
>> +{
>> +	u32 tmp, i, count, maxcount = 0;
>> +
>> +	/* FIXME: this layout was tested only on 2048byte NAND.
>> +	 * NANDs with bigger page size should use more registers. */
> 
> Yep, you should really fix that, or at least reject NAND with a
> different page size until you've fixed that.

ok.

>> +	tmp = ioread32(priv->base + HW_ECC_ERR_CNT);
>> +	for (i = 0; i < 4; i++) {
>> +		count = 0x1f & (tmp >> (5 * i));
>> +		maxcount = max_t(unsigned int, maxcount, count);
>> +	}
>> +
>> +	return count;
>> +}
>> +
>> +static int asm9260_nand_read_page_hwecc(struct mtd_info *mtd,
>> +		struct nand_chip *chip, u8 *buf,
>> +		int oob_required, int page)
>> +{
>> +	struct asm9260_nand_priv *priv = mtd_to_priv(mtd);
>> +	u8 *temp_ptr;
>> +	u32 status, max_bitflips = 0;
>> +
>> +	temp_ptr = buf;
>> +
>> +	/* enable ecc */
>> +	asm9260_reg_rmw(priv, HW_CTRL, BM_CTRL_ECC_EN, 0);
>> +	chip->read_buf(mtd, temp_ptr, mtd->writesize);
>> +
>> +	status = ioread32(priv->base + HW_ECC_CTRL);
>> +
>> +	if (status & BM_ECC_ERR_UNC) {
>> +		u32 ecc_err;
>> +
>> +		ecc_err = ioread32(priv->base + HW_ECC_ERR_CNT);
>> +		/* check if it is erased page (all_DATA_OOB == 0xff) */
>> +		/* FIXME: should be tested if it is a bullet proof solution.
>> +		 *  if not, use is_buf_blank. */
> 
> you'd rather use is_buf_blank if you're unsure.

ok, can is_buf_blank be moved to nand_base?

> 
>> +		if (ecc_err != 0x8421)
>> +			mtd->ecc_stats.failed++;
>> +
>> +	} else if (status & BM_ECC_ERR_CORRECT) {
>> +		max_bitflips = asm9260_nand_count_ecc(priv);
> 
> max_bitflips should contain the maximum number of bitflips in all
> ECC blocks of a given page, here you just returning the number of
> bitflips in the last ECC block.

no. see asm9260_nand_count_ecc.

> The following should do the trick:
> 
> 		int bitflips = asm9260_nand_count_ecc(priv);
> 
> 		if (bitflips > max_bitflips)
> 			max_bitflips = bitflips;
> 
> 		mtd->ecc_stats.corrected += bitflips;
> 
>> +		mtd->ecc_stats.corrected += max_bitflips;
>> +	}
>> +
>> +	if (oob_required)
>> +		chip->ecc.read_oob(mtd, chip, page);
>> +
>> +	return max_bitflips;
>> +}
>> +
>> +static irqreturn_t asm9260_nand_irq(int irq, void *device_info)
>> +{
>> +	struct asm9260_nand_priv *priv = device_info;
>> +	struct nand_chip *nand = &priv->nand;
>> +	u32 status;
>> +
>> +	status = ioread32(priv->base + HW_INT_STATUS);
>> +	if (!status)
>> +		return IRQ_NONE;
>> +
>> +	iowrite32(0, priv->base + HW_INT_MASK);
>> +	iowrite32(0, priv->base + HW_INT_STATUS);
>> +	priv->irq_done = 1;
> 
> You should test the flags before deciding the irq is actually done...

ok. if fixed it by changing mem_mask logic.

> 
>> +	wake_up(&nand->controller->wq);
> 
> and if the condition is not met, don't wake up the waitqueue.
> 
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static void __init asm9260_nand_init_chip(struct nand_chip *nand_chip)
>> +{
>> +	nand_chip->select_chip	= asm9260_select_chip;
>> +	nand_chip->cmdfunc	= asm9260_nand_command_lp;
> 
> You seems to only support large pages NANDs (> 512 bytes), maybe you
> should check if the discovered chip has large pages, and reject the NAND
> otherwise.

ok.

>> +	nand_chip->read_byte	= asm9260_nand_read_byte;
>> +	nand_chip->read_word	= asm9260_nand_read_word;
>> +	nand_chip->read_buf	= asm9260_nand_read_buf;
>> +	nand_chip->write_buf	= asm9260_nand_write_buf;
>> +
>> +	nand_chip->dev_ready	= asm9260_nand_dev_ready;
>> +	nand_chip->chip_delay	= 100;
>> +	nand_chip->options	|= NAND_NO_SUBPAGE_WRITE;
>> +
>> +	nand_chip->ecc.write_page	= asm9260_nand_write_page_hwecc;
>> +	nand_chip->ecc.write_page_raw	= asm9260_nand_write_page_raw;
>> +	nand_chip->ecc.read_page	= asm9260_nand_read_page_hwecc;
>> +}
>> +
>> +static int __init asm9260_nand_cached_config(struct asm9260_nand_priv *priv)
>> +{
>> +	struct nand_chip *nand = &priv->nand;
>> +	struct mtd_info *mtd = &priv->mtd;
>> +	u32 addr_cycles, col_cycles, pages_per_block;
>> +
>> +	pages_per_block = mtd->erasesize / mtd->writesize;
>> +	/* max 256P, min 32P */
>> +	if (pages_per_block & ~(0x000001e0)) {
>> +		dev_err(priv->dev, "Unsupported erasesize 0x%x\n",
>> +				mtd->erasesize);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* max 32K, min 256. */
>> +	if (mtd->writesize & ~(0x0000ff00)) {
>> +		dev_err(priv->dev, "Unsupported writesize 0x%x\n",
>> +				mtd->erasesize);
>> +		return -EINVAL;
>> +	}
>> +
>> +	col_cycles  = 2;
>> +	addr_cycles = col_cycles +
>> +		(((mtd->size >> mtd->writesize) > 65536) ? 3 : 2);
>> +
>> +	priv->ctrl_cache = addr_cycles << BM_CTRL_ADDR_CYCLE1_S
>> +		| BM_CTRL_PAGE_SIZE(mtd->writesize) << BM_CTRL_PAGE_SIZE_S
>> +		| BM_CTRL_BLOCK_SIZE(pages_per_block) << BM_CTRL_BLOCK_SIZE_S
>> +		| BM_CTRL_INT_EN
>> +		| addr_cycles << BM_CTRL_ADDR_CYCLE0_S;
>> +
>> +	iowrite32(BM_ECC_CAPn(nand->ecc.strength) << BM_ECC_CAP_S,
>> +			priv->base + HW_ECC_CTRL);
>> +
>> +	iowrite32(mtd->writesize + priv->spare_size,
>> +			priv->base + HW_ECC_OFFSET);
>> +
>> +	return 0;
>> +}
>> +
>> +static unsigned long __init clk_get_cyc_from_ns(struct clk *clk,
>> +		unsigned long ns)
>> +{
>> +	unsigned int cycle;
>> +
>> +	cycle = NSEC_PER_SEC / clk_get_rate(clk);
>> +	return DIV_ROUND_CLOSEST(ns, cycle);
>> +}
>> +
>> +static void __init asm9260_nand_timing_config(struct asm9260_nand_priv *priv)
>> +{
>> +	struct nand_chip *nand = &priv->nand;
>> +	const struct nand_sdr_timings *time;
>> +	u32 twhr, trhw, trwh, trwp, tadl, tccs, tsync, trr, twb;
>> +	int mode;
>> +
> 
> Maybe you should add support for ONFI timing mode retrieval with
> onfi_get_async_timing_mode.

instead of nand->onfi_timing_mode_default?

>> +	mode = nand->onfi_timing_mode_default;
>> +	dev_info(priv->dev, "ONFI timing mode: %i\n", mode);
>> +
>> +	time = onfi_async_timing_mode_to_sdr_timings(mode);
>> +	if (IS_ERR_OR_NULL(time)) {
>> +		dev_err(priv->dev, "Can't get onfi_timing_mode: %i\n", mode);
>> +		return;
>> +	}
>> +
>> +	trwh = clk_get_cyc_from_ns(priv->clk, time->tWH_min / 1000);
>> +	trwp = clk_get_cyc_from_ns(priv->clk, time->tWP_min / 1000);
>> +
>> +	iowrite32((trwh << 4) | (trwp), priv->base + HW_TIMING_ASYN);
>> +
>> +	twhr = clk_get_cyc_from_ns(priv->clk, time->tWHR_min / 1000);
>> +	trhw = clk_get_cyc_from_ns(priv->clk, time->tRHW_min / 1000);
>> +	tadl = clk_get_cyc_from_ns(priv->clk, time->tADL_min / 1000);
>> +	/* tCCS - change read/write collumn. Time between last cmd and data. */
>> +	tccs = clk_get_cyc_from_ns(priv->clk,
>> +			(time->tCLR_min + time->tCLH_min + time->tRC_min)
>> +			/ 1000);
>> +
>> +	iowrite32((twhr << 24) | (trhw << 16)
>> +		| (tadl << 8) | (tccs), priv->base + HW_TIM_SEQ_0);
>> +
>> +	trr = clk_get_cyc_from_ns(priv->clk, time->tRR_min / 1000);
>> +	tsync = 0; /* ??? */
>> +	twb = clk_get_cyc_from_ns(priv->clk, time->tWB_max / 1000);
>> +	iowrite32((tsync << 16) | (trr << 9) | (twb),
>> +			priv->base + HW_TIM_SEQ_1);
>> +}
>> +
>> +static int __init asm9260_nand_ecc_conf(struct asm9260_nand_priv *priv)
>> +{
>> +	struct device_node *np = priv->dev->of_node;
>> +	struct nand_chip *nand = &priv->nand;
>> +	struct mtd_info *mtd = &priv->mtd;
>> +	struct nand_ecclayout *ecc_layout = &priv->ecc_layout;
>> +	int i, ecc_strength;
>> +
>> +	nand->ecc.mode = of_get_nand_ecc_mode(np);
>> +	switch (nand->ecc.mode) {
>> +	case NAND_ECC_SOFT:
>> +	case NAND_ECC_SOFT_BCH:
>> +		dev_info(priv->dev, "Using soft ECC %i\n", nand->ecc.mode);
>> +		/* nand_base will find needed settings */
>> +		return 0;
>> +	case NAND_ECC_HW:
>> +	default:
>> +		dev_info(priv->dev, "Using default NAND_ECC_HW\n");
>> +		nand->ecc.mode = NAND_ECC_HW;
>> +		break;
>> +	}
>> +
>> +	ecc_strength = of_get_nand_ecc_strength(np);
>> +	nand->ecc.size = ASM9260_ECC_STEP;
>> +	nand->ecc.steps = mtd->writesize / nand->ecc.size;
>> +
>> +	if (ecc_strength < nand->ecc_strength_ds) {
>> +		int ds_corr;
>> +
>> +		/* Let's check if ONFI can help us. */
>> +		if (nand->ecc_strength_ds <= 0) {
> 
> Actually this is not necessarily filled by ONFI parameters (it can be
> statically defined in the nand_ids table).

Should i sepcify it by dev_err or enough to say it in comment?

>> +			/* No ONFI and no DT - it is bad. */
>> +			dev_err(priv->dev,
>> +					"nand-ecc-strength is not set by DT or ONFI. Please set nand-ecc-strength in DT or add chip quirk in nand_ids.c.\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		ds_corr = (mtd->writesize * nand->ecc_strength_ds) /
>> +			nand->ecc_step_ds;
>> +		ecc_strength = ds_corr / nand->ecc.steps;
>> +		dev_info(priv->dev, "ONFI:nand-ecc-strength = %i\n",
>> +				ecc_strength);
>> +	} else
>> +		dev_info(priv->dev, "DT:nand-ecc-strength = %i\n",
>> +				ecc_strength);
>> +
>> +	if (ecc_strength == 0 || ecc_strength > ASM9260_ECC_MAX_BIT) {
>> +		dev_err(priv->dev,
>> +				"Not supported ecc_strength value: %i\n",
>> +				ecc_strength);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (ecc_strength & 0x1) {
>> +		ecc_strength++;
>> +		dev_info(priv->dev,
>> +				"Only even ecc_strength value is supported. Recalculating: %i\n",
>> +		       ecc_strength);
>> +	}
>> +
>> +	/* FIXME: do we have max or min page size? */
>> +
>> +	/* 13 - the smallest integer for 512 (ASM9260_ECC_STEP). Div to 8bit. */
>> +	nand->ecc.bytes = DIV_ROUND_CLOSEST(ecc_strength * 13, 8);
>> +
>> +	ecc_layout->eccbytes = nand->ecc.bytes * nand->ecc.steps;
>> +	nand->ecc.layout = ecc_layout;
>> +	nand->ecc.strength = ecc_strength;
>> +
>> +	priv->spare_size = mtd->oobsize - ecc_layout->eccbytes;
>> +
>> +	ecc_layout->oobfree[0].offset = 2;
>> +	ecc_layout->oobfree[0].length = priv->spare_size - 2;
>> +
>> +	/* FIXME: can we use same layout as SW_ECC? */
> 
> It depends on what your controller is capable of. If you can define the
> offset at which you write the ECC bytes, then yes you can reuse the
> same kind of layout used in SW_ECC.
> 
>> +	for (i = 0; i < ecc_layout->eccbytes; i++)
>> +		ecc_layout->eccpos[i] = priv->spare_size + i;
>> +
>> +	return 0;
>> +}
>> +
>> +static int __init asm9260_nand_get_dt_clks(struct asm9260_nand_priv *priv)
>> +{
>> +	int clk_idx = 0, err;
>> +
>> +	priv->clk = devm_clk_get(priv->dev, "sys");
>> +	if (IS_ERR(priv->clk))
>> +		goto out_err;
>> +
>> +	/* configure AHB clock */
>> +	clk_idx = 1;
>> +	priv->clk_ahb = devm_clk_get(priv->dev, "ahb");
>> +	if (IS_ERR(priv->clk_ahb))
>> +		goto out_err;
>> +
>> +	err = clk_prepare_enable(priv->clk_ahb);
>> +	if (err)
>> +		dev_err(priv->dev, "Failed to enable ahb_clk!\n");
>> +
>> +	err = clk_set_rate(priv->clk, clk_get_rate(priv->clk_ahb));
>> +	if (err) {
>> +		clk_disable_unprepare(priv->clk_ahb);
>> +		dev_err(priv->dev, "Failed to set rate!\n");
>> +	}
>> +
>> +	err = clk_prepare_enable(priv->clk);
>> +	if (err) {
>> +		clk_disable_unprepare(priv->clk_ahb);
>> +		dev_err(priv->dev, "Failed to enable clk!\n");
>> +	}
>> +
>> +	return 0;
>> +out_err:
>> +	dev_err(priv->dev, "%s: Failed to get clk (%i)\n", __func__, clk_idx);
>> +	return 1;
>> +}
>> +
>> +static int __init asm9260_nand_probe(struct platform_device *pdev)
>> +{
>> +	struct asm9260_nand_priv *priv;
>> +	struct nand_chip *nand;
>> +	struct mtd_info *mtd;
>> +	struct device_node *np = pdev->dev.of_node;
>> +	struct resource *r;
>> +	int ret, i;
>> +	unsigned int irq;
>> +	u32 val;
>> +
>> +	priv = devm_kzalloc(&pdev->dev, sizeof(struct asm9260_nand_priv),
>> +			GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	priv->base = devm_ioremap_resource(&pdev->dev, r);
>> +	if (!priv->base) {
>> +		dev_err(&pdev->dev, "Unable to map resource!\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	priv->dev = &pdev->dev;
>> +	nand = &priv->nand;
>> +	nand->priv = priv;
>> +
>> +	platform_set_drvdata(pdev, priv);
>> +	mtd = &priv->mtd;
>> +	mtd->priv = nand;
>> +	mtd->owner = THIS_MODULE;
>> +	mtd->name = dev_name(&pdev->dev);
>> +
>> +	if (asm9260_nand_get_dt_clks(priv))
>> +		return -ENODEV;
>> +
>> +	irq = platform_get_irq(pdev, 0);
>> +	if (!irq)
>> +		return -ENODEV;
>> +
>> +	iowrite32(0, priv->base + HW_INT_MASK);
>> +	ret = devm_request_irq(priv->dev, irq, asm9260_nand_irq,
>> +				IRQF_ONESHOT | IRQF_SHARED,
>> +				dev_name(&pdev->dev), priv);
>> +
>> +	asm9260_nand_init_chip(nand);
>> +
>> +	ret = of_property_read_u32(np, "nand-max-chips", &val);
>> +	if (ret)
>> +		val = 1;
>> +
>> +	if (val > ASM9260_MAX_CHIPS) {
>> +		dev_err(&pdev->dev, "Unsupported nand-max-chips value: %i\n",
>> +				val);
>> +		return -ENODEV;
>> +	}
>> +
>> +	for (i = 0; i < val; i++)
>> +		priv->mem_mask |= BM_CTRL_MEM0_RDY << i;
> 
> If you want to support multiple NAND chips, then I recommend to rework
> the DT representation and to define a asm9260_nand_controller struct:
> 
> struct asm9260_nand_controller {
> 	struct nand_hw_control base;
> 
> 	/* asm9260 related stuff */
> };
> 
> Take a look [2] and [3].


need to tak clother look. Right now i don't understand meaning of struct
nand_hw_control.

> 
>> +
>> +	ret = nand_scan_ident(mtd, val, NULL);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "scan_ident filed!\n");
>> +		return ret;
>> +	}
>> +
>> +	if (of_get_nand_on_flash_bbt(np)) {
>> +		dev_info(&pdev->dev, "Use On Flash BBT\n");
>> +		nand->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB_BBM
>> +			| NAND_BBT_LASTBLOCK;
>> +	}
>> +
>> +	asm9260_nand_timing_config(priv);
>> +	asm9260_nand_ecc_conf(priv);
>> +	ret = asm9260_nand_cached_config(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* second phase scan */
>> +	if (nand_scan_tail(mtd)) {
>> +		dev_err(&pdev->dev, "scan_tail filed!\n");
>> +		return -ENXIO;
>> +	}
>> +
>> +
>> +	ret = mtd_device_parse_register(mtd, NULL,
>> +			&(struct mtd_part_parser_data) {
>> +				.of_node = pdev->dev.of_node,
>> +			},
>> +			NULL, 0);
> 
> Ergh, can you please define a local variable to replace this ugly
> mtd_part_parser_data definition ?

why?

-- 
Regards,
Oleksij

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20150101/7417ce43/attachment.sig>


More information about the linux-mtd mailing list