Hi Randolph, On 04.06.2018 19:16, Randolph Maaßen wrote: > Am Freitag, den 01.06.2018, 00:16 +0200 schrieb Stefan Agner: >> 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 | 1143 >> +++++++++++++++++++++++++++++ >> 4 files changed, 1157 insertions(+) >> create mode 100644 drivers/mtd/nand/raw/tegra_nand.c >> [...] >> +static int tegra_nand_chips_init(struct device *dev, >> + struct tegra_nand_controller *ctrl) >> +{ >> + struct device_node *np = dev->of_node; >> + struct device_node *np_nand; >> + int nchips = of_get_child_count(np); >> + struct tegra_nand_chip *nand; >> + struct mtd_info *mtd; >> + struct nand_chip *chip; >> + unsigned long config, bch_config = 0; >> + int bits_per_step; >> + int ret; >> + >> + if (nchips != 1) { >> + dev_err(dev, "Currently only one NAND chip >> supported\n"); >> + return -EINVAL; >> + } >> + >> + np_nand = of_get_next_child(np, NULL); >> + >> + nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL); >> + if (!nand) >> + return -ENOMEM; >> + >> + nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", >> GPIOD_OUT_LOW); >> + >> + if (IS_ERR(nand->wp_gpio)) { >> + ret = PTR_ERR(nand->wp_gpio); >> + dev_err(dev, "Failed to request WP GPIO: %d\n", >> ret); >> + return ret; >> + } >> + >> + chip = &nand->chip; >> + chip->controller = &ctrl->controller; >> + >> + mtd = nand_to_mtd(chip); >> + >> + mtd->dev.parent = dev; >> + if (!mtd->name) >> + mtd->name = "tegra_nand"; >> + mtd->owner = THIS_MODULE; >> + >> + nand_set_flash_node(chip, np_nand); > > Hi, > i just tried this driver and it works great so far, thanks. > I just found, that assigning the of node after setting the mtd->name > makes it impossible to assign a name via devicetree label. I have read > the discussion about the label on this list, so I'm curious if this is > intentional? Setting mtd->name after nand_set_flash_node() enables the > label parameter. > Hm, good catch. No that was not intentional. The name indeed should be assigned after the call to nand_set_flash_node. Will fix this in the next revision. >> + >> + chip->options = NAND_NO_SUBPAGE_WRITE | >> NAND_USE_BOUNCE_BUFFER; >> + chip->exec_op = tegra_nand_exec_op; >> + chip->select_chip = tegra_nand_select_chip; >> + chip->setup_data_interface = >> tegra_nand_setup_data_interface; >> + >> + ret = nand_scan_ident(mtd, 1, NULL); >> + if (ret) >> + return ret; >> + >> + if (chip->bbt_options & NAND_BBT_USE_FLASH) >> + chip->bbt_options |= NAND_BBT_NO_OOB; >> + >> + chip->ecc.mode = NAND_ECC_HW; >> + chip->ecc.size = 512; >> + chip->ecc.steps = mtd->writesize / chip->ecc.size; >> + if (chip->ecc_step_ds != 512) { >> + dev_err(dev, "Unsupported step size %d\n", chip- >> >ecc_step_ds); >> + return -EINVAL; >> + } >> + >> + chip->ecc.read_page = tegra_nand_read_page_hwecc; >> + chip->ecc.write_page = tegra_nand_write_page_hwecc; >> + >> + config = readl_relaxed(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; >> + >> + if (chip->ecc.algo == NAND_ECC_UNKNOWN) { >> + if (mtd->writesize < 2048) >> + chip->ecc.algo = NAND_ECC_RS; >> + else >> + chip->ecc.algo = NAND_ECC_BCH; >> + } >> + >> + if (chip->ecc.algo == NAND_ECC_BCH && mtd->writesize < 2048) >> { >> + dev_err(dev, "BCH supportes 2K or 4K page size >> only\n"); >> + return -EINVAL; >> + } >> + >> + if (!chip->ecc.strength) { >> + ret = tegra_nand_select_strength(chip, mtd- >> >oobsize); >> + if (ret < 0) { >> + dev_err(dev, "No valid strenght found, >> minimum %d\n", >> + chip->ecc_strength_ds); >> + return ret; >> + } >> + >> + chip->ecc.strength = ret; >> + } >> + >> + 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; >> + } >> + >> + dev_info(dev, "Using %s with strength %d per 512 byte >> step\n", >> + chip->ecc.algo == NAND_ECC_BCH ? "BCH" : >> "RS", >> + chip->ecc.strength); >> + >> + chip->ecc.bytes = DIV_ROUND_UP(bits_per_step, >> BITS_PER_BYTE); >> + >> + 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, "Unsupported writesize %d\n", mtd- >> >writesize); >> + return -ENODEV; >> + } >> + >> + writel_relaxed(config, ctrl->regs + CFG); >> + writel_relaxed(bch_config, ctrl->regs + BCH_CONFIG); >> + >> + ret = nand_scan_tail(mtd); >> + if (ret) >> + return ret; >> + >> + mtd_ooblayout_free(mtd, 0, &nand->tag); >> + >> + config |= CFG_TAG_BYTE_SIZE(nand->tag.length - 1); >> + writel_relaxed(config, ctrl->regs + CFG); >> + >> + ret = mtd_device_register(mtd, NULL, 0); >> + if (ret) { >> + dev_err(dev, "Failed to register mtd device: %d\n", >> ret); >> + nand_cleanup(chip); >> + return ret; >> + } >> + >> + ctrl->chip = chip; >> + >> + 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 reg; >> + 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; >> + >> + err = reset_control_reset(rst); >> + if (err) >> + goto err_disable_clk; >> + >> + reg = HWSTATUS_RDSTATUS_MASK(1) | HWSTATUS_RDSTATUS_VALUE(0) >> | >> + HWSTATUS_RBSY_MASK(NAND_STATUS_READY) | >> + HWSTATUS_RBSY_VALUE(NAND_STATUS_READY); >> + writel_relaxed(NAND_CMD_STATUS, ctrl->regs + HWSTATUS_CMD); >> + writel_relaxed(reg, ctrl->regs + HWSTATUS_MASK); >> + >> + init_completion(&ctrl->command_complete); >> + init_completion(&ctrl->dma_complete); >> + >> + /* clear interrupts */ >> + reg = readl_relaxed(ctrl->regs + ISR); >> + writel_relaxed(reg, 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_relaxed(DMA_CTRL_IS_DONE, ctrl->regs + DMA_CTRL); >> + >> + /* enable interrupts */ >> + reg = IER_UND | IER_OVR | IER_CMD_DONE | IER_GIE; >> + writel_relaxed(reg, ctrl->regs + IER); >> + >> + /* reset config */ >> + writel_relaxed(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); > > Sorry for any noise/mistake, I'm new to kernel development That was constructive feedback! Thanks for pointing out! -- Stefan -- 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