Re: [PATCH 12/15] mtd: nand: add Ambarella nand support

Miquel Raynal
2023-01-23T08:33:10Z

Hi Li,

I'm sorry, this is not going to work at all.

> + ambarella_nand_init(host);
> +
> + mtd = nand_to_mtd(&host->chip);
> + mtd->name = "amba_nand";
> +
> + nand_controller_init(&host->controller);
> + nand_set_controller_data(&host->chip, host);
> + nand_set_flash_node(&host->chip, dev->of_node);
> +
> + host->chip.controller = &host->controller;
> + host->chip.controller->ops = &ambarella_controller_ops;
> + host->chip.legacy.chip_delay = 0;
> + host->chip.legacy.read_byte = ambarella_nand_read_byte;
> + host->chip.legacy.write_buf = ambarella_nand_write_buf;
> + host->chip.legacy.read_buf = ambarella_nand_read_buf;
> + host->chip.legacy.select_chip = ambarella_nand_select_chip;
> + host->chip.legacy.cmd_ctrl = ambarella_nand_cmd_ctrl;
> + host->chip.legacy.dev_ready = ambarella_nand_dev_ready;
> + host->chip.legacy.waitfunc = ambarella_nand_waitfunc;
> + host->chip.legacy.cmdfunc = ambarella_nand_cmdfunc;
> + host->chip.legacy.set_features = nand_get_set_features_notsupp;
> + host->chip.legacy.get_features = nand_get_set_features_notsupp;

Please be aware that we no longer accept legacy introductions upstream.

You can look for ->exec_op() conversions using git-log.

> + host->chip.options |= NAND_NO_SUBPAGE_WRITE | NAND_USES_DMA;
> +
> + rval = nand_scan(&host->chip, 1);
> + if (rval < 0)
> + return rval;
> +
> + rval = mtd_device_register(mtd, NULL, 0);
> + if (rval < 0)
> + nand_cleanup(&host->chip);
> +
> + return rval;
> +}

[...]

> diff --git a/drivers/mtd/nand/raw/nand_ids.c
b/drivers/mtd/nand/raw/nand_ids.c
> index dacc5529b3df..9f264e2a6484 100644
> --- a/drivers/mtd/nand/raw/nand_ids.c
> +++ b/drivers/mtd/nand/raw/nand_ids.c
> @@ -62,6 +62,10 @@ struct nand_flash_dev nand_flash_ids[] = {
> { .id = {0x98, 0xd3, 0x91, 0x26, 0x76} },
> SZ_4K, SZ_1K, SZ_256K, 0, 5, 256, NAND_ECC_INFO(8, SZ_512)},
>
> + {"MT29F2G01ABAGD SPINAND 2G 3.3V 8-bit",
> + { .id = {0x2c, 0x24, 0x00, 0x00, 0x00} },
> + SZ_2K, SZ_256, SZ_128K, 0, 2, 128},
> +

Raw NAND != SPI-NAND. I don't get what you're doing here but either you
want to drive SPI-NANDs and this is a SPI controller driver that
implements spi-mem ops and should be located under drivers/spi/, or
this is a plain raw NAND controller which is wired to a parallel NAND
and this should be under drivers/mtd/nand/raw/.

ECC controllers can be shared with the ECC engine abstraction though.

> LEGACY_ID_NAND("NAND 4MiB 5V 8-bit", 0x6B, 4, SZ_8K, SP_OPTIONS),
> LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 4, SZ_8K, SP_OPTIONS),
> LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE5, 4, SZ_8K, SP_OPTIONS),

Thanks,
Miquèl