On 31.05.2018 11:37, Stefan Agner wrote: > On 27.05.2018 23:54, Stefan Agner wrote: >> Add support for the NAND flash controller found on NVIDIA >> Tegra 2 SoCs. This implementation does not make use of the >> command queue feature. Regular operations/data transfers are >> done in PIO mode. Page read/writes with hardware ECC make >> use of the DMA for data transfer. >> >> Signed-off-by: Lucas Stach <dev@xxxxxxxxxx> >> Signed-off-by: Stefan Agner <stefan@xxxxxxxx> >> --- >> MAINTAINERS | 7 + >> drivers/mtd/nand/raw/Kconfig | 6 + >> drivers/mtd/nand/raw/Makefile | 1 + >> drivers/mtd/nand/raw/tegra_nand.c | 999 ++++++++++++++++++++++++++++++ >> 4 files changed, 1013 insertions(+) >> create mode 100644 drivers/mtd/nand/raw/tegra_nand.c >> > [...] >> + >> + chip->ecc.read_page = tegra_nand_read_page_hwecc; >> + chip->ecc.write_page = tegra_nand_write_page_hwecc; >> + /* Not functional for unknown reason... >> + chip->ecc.read_page_raw = tegra_nand_read_page; >> + chip->ecc.write_page_raw = tegra_nand_write_page; >> + */ > > I am giving up on these raw read/write_page functions. Using DMA without > HW ECC just seems not to work. > > I do not get the CMD_DONE interrupt as I do when using DMA with HW ECC. > I tried with the same settings, just not enabling DMA (HW_ECC not set, > HW_ERR_CORRECTION not set, and also with/without PIPELINE_EN). s/not enabling DMA/not enabling HW ECC/ > > A register dump after a successful read with HW ECC looks like this: > [ 196.935199] CMD: 0x66890104 > [ 196.938003] STATUS: 0x00000101 > [ 196.941049] ISR: 0x00000000 > [ 196.943865] CONFIG: 0x0084000b > [ 196.946914] DMA_MST_CTRL: 0x15000004 > [ 196.950481] DMA_CFG_A: 0x00000fff > [ 196.954361] DMA_CFG_B: 0x00000000 > [ 196.957670] FIFO: 0x0000aa00 > > Whereas without HW ECC completion times out (using > wait_for_completion_timeout already): > [ 226.128618] tegra-nand 70008000.nand: CMD timeout, last CMD: > 0xe6890104 > [ 226.135375] CMD: 0xe6890104 > [ 226.138201] STATUS: 0x00000100 > [ 226.141280] ISR: 0x00000100 > [ 226.153372] CONFIG: 0x0084000b > [ 226.156423] DMA_MST_CTRL: 0x95000004 > [ 226.159989] DMA_CFG_A: 0x00000fff > [ 226.164084] DMA_CFG_B: 0x00000000 > [ 226.167393] FIFO: 0x0000aa00 > > It looks to me as if the DMA just does not start the data cycle. The > NAND seems to have read the page (RBSY0 is set). > > > Note that it is never explicitly stated whether DMA without HW ECC is > supported. There is some indication that it should: There are separated > bits to enable HW ECC/DMA, the reference manual states "If HW ECC is > enabled... " and a block diagram shows separate blocks for the ECC > Engine and NAND DMA control. > > There is also indication that it does not: The chapter Restrictions > reads: "HW_ERR_CORRECTION = 0/1, doesn’t alter controller hardware > behavior. Software error correction scheme with HW_ERR_CORRECTION = 0, > is deprecated in Tegra 2 Processor." > > Note that the default implementations nand_(read|write)_page_raw which > use exec_op do work fine! Unfortunately, the PIO mode only allows 4 > bytes in a read cycle, hence raw read/write is slow... > > -- > Stefan > > >> + config = readl(ctrl->regs + CFG); >> + config |= CFG_PIPE_EN | CFG_SKIP_SPARE | CFG_SKIP_SPARE_SIZE_4; >> + >> + if (chip->options & NAND_BUSWIDTH_16) >> + config |= CFG_BUS_WIDTH_16; >> + >> + switch (chip->ecc.algo) { >> + case NAND_ECC_RS: >> + bits_per_step = BITS_PER_STEP_RS * chip->ecc.strength; >> + mtd_set_ooblayout(mtd, &tegra_nand_oob_rs_ops); >> + switch (chip->ecc.strength) { >> + case 4: >> + config |= CFG_ECC_SEL | CFG_TVAL_4; >> + break; >> + case 6: >> + config |= CFG_ECC_SEL | CFG_TVAL_6; >> + break; >> + case 8: >> + config |= CFG_ECC_SEL | CFG_TVAL_8; >> + break; >> + default: >> + dev_err(dev, "ECC strength %d not supported\n", >> + chip->ecc.strength); >> + return -EINVAL; >> + } >> + break; >> + case NAND_ECC_BCH: >> + bits_per_step = BITS_PER_STEP_BCH * chip->ecc.strength; >> + mtd_set_ooblayout(mtd, &tegra_nand_oob_bch_ops); >> + switch (chip->ecc.strength) { >> + case 4: >> + bch_config = BCH_TVAL_4; >> + break; >> + case 8: >> + bch_config = BCH_TVAL_8; >> + break; >> + case 14: >> + bch_config = BCH_TVAL_14; >> + break; >> + case 16: >> + bch_config = BCH_TVAL_16; >> + break; >> + default: >> + dev_err(dev, "ECC strength %d not supported\n", >> + chip->ecc.strength); >> + return -EINVAL; >> + } >> + break; >> + default: >> + dev_err(dev, "ECC algorithm not supported\n"); >> + return -EINVAL; >> + } >> + >> + chip->ecc.bytes = DIV_ROUND_UP(bits_per_step, 8); >> + >> + switch (mtd->writesize) { >> + case 256: >> + config |= CFG_PS_256; >> + break; >> + case 512: >> + config |= CFG_PS_512; >> + break; >> + case 1024: >> + config |= CFG_PS_1024; >> + break; >> + case 2048: >> + config |= CFG_PS_2048; >> + break; >> + case 4096: >> + config |= CFG_PS_4096; >> + break; >> + default: >> + dev_err(dev, "unhandled writesize %d\n", mtd->writesize); >> + return -ENODEV; >> + } >> + >> + writel(config, ctrl->regs + CFG); >> + writel(bch_config, ctrl->regs + BCH_CONFIG); >> + >> + err = nand_scan_tail(mtd); >> + if (err) >> + return err; >> + >> + config |= CFG_TAG_BYTE_SIZE(mtd_ooblayout_count_freebytes(mtd) - 1); >> + writel(config, ctrl->regs + CFG); >> + >> + err = mtd_device_register(mtd, NULL, 0); >> + if (err) >> + return err; >> + >> + return 0; >> +} >> + >> +static int tegra_nand_probe(struct platform_device *pdev) >> +{ >> + struct reset_control *rst; >> + struct tegra_nand_controller *ctrl; >> + struct resource *res; >> + unsigned long value; >> + int irq, err = 0; >> + >> + ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL); >> + if (!ctrl) >> + return -ENOMEM; >> + >> + ctrl->dev = &pdev->dev; >> + nand_hw_control_init(&ctrl->controller); >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + ctrl->regs = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(ctrl->regs)) >> + return PTR_ERR(ctrl->regs); >> + >> + rst = devm_reset_control_get(&pdev->dev, "nand"); >> + if (IS_ERR(rst)) >> + return PTR_ERR(rst); >> + >> + ctrl->clk = devm_clk_get(&pdev->dev, "nand"); >> + if (IS_ERR(ctrl->clk)) >> + return PTR_ERR(ctrl->clk); >> + >> + err = clk_prepare_enable(ctrl->clk); >> + if (err) >> + return err; >> + >> + reset_control_reset(rst); >> + >> + value = HWSTATUS_RDSTATUS_MASK(1) | HWSTATUS_RDSTATUS_VALUE(0) | >> + HWSTATUS_RBSY_MASK(NAND_STATUS_READY) | >> + HWSTATUS_RBSY_VALUE(NAND_STATUS_READY); >> + writel(NAND_CMD_STATUS, ctrl->regs + HWSTATUS_CMD); >> + writel(value, ctrl->regs + HWSTATUS_MASK); >> + >> + init_completion(&ctrl->command_complete); >> + init_completion(&ctrl->dma_complete); >> + >> + /* clear interrupts */ >> + value = readl(ctrl->regs + ISR); >> + writel(value, ctrl->regs + ISR); >> + >> + irq = platform_get_irq(pdev, 0); >> + err = devm_request_irq(&pdev->dev, irq, tegra_nand_irq, 0, >> + dev_name(&pdev->dev), ctrl); >> + if (err) >> + goto err_disable_clk; >> + >> + writel(DMA_CTRL_IS_DONE, ctrl->regs + DMA_CTRL); >> + >> + /* enable interrupts */ >> + value = IER_UND | IER_OVR | IER_CMD_DONE | IER_ECC_ERR | IER_GIE; >> + writel(value, ctrl->regs + IER); >> + >> + /* reset config */ >> + writel(0, ctrl->regs + CFG); >> + >> + err = tegra_nand_chips_init(ctrl->dev, ctrl); >> + if (err) >> + goto err_disable_clk; >> + >> + platform_set_drvdata(pdev, ctrl); >> + >> + return 0; >> + >> +err_disable_clk: >> + clk_disable_unprepare(ctrl->clk); >> + return err; >> +} >> + >> +static int tegra_nand_remove(struct platform_device *pdev) >> +{ >> + struct tegra_nand_controller *ctrl = platform_get_drvdata(pdev); >> + >> + nand_release(nand_to_mtd(ctrl->chip)); >> + >> + clk_disable_unprepare(ctrl->clk); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id tegra_nand_of_match[] = { >> + { .compatible = "nvidia,tegra20-nand" }, >> + { /* sentinel */ } >> +}; >> + >> +static struct platform_driver tegra_nand_driver = { >> + .driver = { >> + .name = "tegra-nand", >> + .of_match_table = tegra_nand_of_match, >> + }, >> + .probe = tegra_nand_probe, >> + .remove = tegra_nand_remove, >> +}; >> +module_platform_driver(tegra_nand_driver); >> + >> +MODULE_DESCRIPTION("NVIDIA Tegra NAND driver"); >> +MODULE_AUTHOR("Thierry Reding <thierry.reding@xxxxxxxxxx>"); >> +MODULE_AUTHOR("Lucas Stach <dev@xxxxxxxxxx>"); >> +MODULE_AUTHOR("Stefan Agner <stefan@xxxxxxxx>"); >> +MODULE_LICENSE("GPL v2"); >> +MODULE_DEVICE_TABLE(of, tegra_nand_of_match); -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html