On Mon, Aug 19, 2024 at 09:20:37AM +0000, Hui-Ping Chen wrote:

Nuvoton MA35 SoCs NAND Flash Interface Controller supports 2KB, 4KB and 8KB page size, and up to 8-bit, 12-bit, and 24-bit hardware ECC calculation circuit to protect data communication.

Signed-off-by: Hui-Ping Chen hpchen0nvt@gmail.com

...

+static int ma35_nand_probe(struct platform_device *pdev) +{

• struct ma35_nand_info *nand;
• struct nand_chip *chip;
• struct mtd_info *mtd;
• int retval = 0;
• nand = devm_kzalloc(&pdev->dev, sizeof(*nand), GFP_KERNEL);
• if (!nand)

• return -ENOMEM;
  

• nand_controller_init(&nand->controller);
• nand->regs = devm_platform_ioremap_resource(pdev, 0);
• if (IS_ERR(nand->regs))

• return PTR_ERR(nand->regs);
  

• nand->dev = &pdev->dev;
• chip = &nand->chip;
• mtd = nand_to_mtd(chip);
• nand_set_controller_data(chip, nand);
• nand_set_flash_node(chip, pdev->dev.of_node);
• mtd->priv = chip;
• mtd->owner = THIS_MODULE;
• mtd->dev.parent = &pdev->dev;
• nand->clk = devm_clk_get(&pdev->dev, "nand_gate");
• if (IS_ERR(nand->clk))

• return dev_err_probe(&pdev->dev, PTR_ERR(nand->clk),
  

• 		     "failed to find nand clock\n");
  

• retval = clk_prepare_enable(nand->clk);
• if (retval < 0) {

• dev_err(&pdev->dev, "failed to enable clock\n");
  

• retval = -ENXIO;
  

• }
• nand->chip.controller = &nand->controller;
• chip->legacy.cmdfunc = ma35_nand_command;
• chip->legacy.waitfunc = ma35_waitfunc;
• chip->legacy.read_byte = ma35_nand_read_byte;
• chip->legacy.select_chip = ma35_nand_select_chip;
• chip->legacy.read_buf = ma35_read_buf_dma;
• chip->legacy.write_buf = ma35_write_buf_dma;
• chip->legacy.dev_ready = ma35_nand_devready;
• chip->legacy.chip_delay = 25; /* us */
• /* Read OOB data first, then HW read page */
• chip->ecc.hwctl = ma35_nand_enable_hwecc;
• chip->ecc.calculate = ma35_nand_calculate_ecc;
• chip->ecc.correct = ma35_nand_correct_data;
• chip->ecc.write_page = ma35_nand_write_page_hwecc;
• chip->ecc.read_page = ma35_nand_read_page_hwecc_oob_first;
• chip->ecc.read_oob = ma35_nand_read_oob_hwecc;
• chip->options |= (NAND_NO_SUBPAGE_WRITE | NAND_USES_DMA);
• ma35_nand_initialize(nand);
• platform_set_drvdata(pdev, nand);
• nand->controller.ops = &ma35_nand_controller_ops;
• nand->irq = platform_get_irq(pdev, 0);
• if (nand->irq < 0)

• return dev_err_probe(&pdev->dev, nand->irq,
  

• 		     "failed to get platform irq\n");
  

• if (request_irq(nand->irq, ma35_nand_irq, IRQF_TRIGGER_HIGH, "ma35d1-nand", nand)) {

• dev_err(&pdev->dev, "Error requesting NAND IRQ\n");
  

• return -ENXIO;
  

• }
• retval = nand_scan(chip, 1);
• if (retval)

• return retval;
  

• if (mtd_device_register(mtd, nand->parts, nand->nr_parts)) {

• nand_cleanup(chip);
  

• devm_kfree(&pdev->dev, nand);
  

• return retval;
  

• }
• return retval;

+}

+static void ma35_nand_remove(struct platform_device *pdev) +{

• struct ma35_nand_info *nand = platform_get_drvdata(pdev);
• struct nand_chip *chip = &nand->chip;
• int ret;

Where do you release IRQ handler?

• ret = mtd_device_unregister(nand_to_mtd(chip));
• WARN_ON(ret);
• nand_cleanup(chip);
• clk_disable_unprepare(nand->clk);
• kfree(nand);

NAK, you never tested your code.

• platform_set_drvdata(pdev, NULL);

Why? Drop.

+}

+/* PM Support */ +#ifdef CONFIG_PM +static int ma35_nand_suspend(struct platform_device *pdev, pm_message_t pm) +{

• struct ma35_nand_info *nand = platform_get_drvdata(pdev);
• unsigned long timeo = jiffies + HZ/2;
• /* wait DMAC to ready */
• while (1) {

• if ((readl(nand->regs + MA35_NFI_REG_DMACTL) & DMA_BUSY) == 0)
  

• 	break;
  

• if (time_after(jiffies, timeo))
  

• 	return -ETIMEDOUT;
  

• }
• clk_disable(nand->clk);
• return 0;

+}

+static int ma35_nand_resume(struct platform_device *pdev) +{

• struct ma35_nand_info *nand = platform_get_drvdata(pdev);
• clk_enable(nand->clk);
• ma35_nand_hwecc_init(nand);
• ma35_nand_dmac_init(nand);
• return 0;

+}

+#else +#define ma35_nand_suspend NULL +#define ma35_nand_resume NULL +#endif

+static const struct of_device_id ma35_nfi_of_match[] = {

• { .compatible = "nuvoton,ma35d1-nand" },
• {},

+}; +MODULE_DEVICE_TABLE(of, ma35_nfi_of_match);

+static struct platform_driver ma35_nand_driver = {

• .driver = {
  

Messed indentation.

• .name   = "ma35d1-nand",
  

• .owner  = THIS_MODULE,
  

Drop. Please do not upstream some 10 year old code... Use recent code as template, not 10yo stuff...

Best regards, Krzysztof