| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <t7igmxzylpem6qbasb2esnren743m4dnzw6j3i7vn3dveubu5n@4sy6mmmlhtw3>
Date: Mon, 19 Aug 2024 13:19:00 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Hui-Ping Chen <hpchen0nvt@...il.com>
Cc: miquel.raynal@...tlin.com, richard@....at, vigneshr@...com,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, sumit.semwal@...aro.org,
christian.koenig@....com, esben@...nix.com, linux-arm-kernel@...ts.infradead.org,
linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v2 2/2] mtd: rawnand: nuvoton: add new driver for the
Nuvoton MA35 SoC
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@...il.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
Powered by blists - more mailing lists