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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Mon, 23 Jan 2023 09:32:09 +0100
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Li Chen <lchen@...arella.com>
Cc:     Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Li Chen <me@...ux.beauty>, Roger Quadros <rogerq@...nel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Chuanhong Guo <gch981213@...il.com>,
        Liang Yang <liang.yang@...ogic.com>,
        Jean Delvare <jdelvare@...e.de>,
        Andreas Böhler <dev@...ehler.at>,
        Christian Lamparter <chunkeey@...il.com>,
        Rickard x Andersson <rickaran@...s.com>,
        linux-kernel@...r.kernel.org (open list),
        linux-mtd@...ts.infradead.org (open list:NAND FLASH SUBSYSTEM),
        linux-arm-kernel@...ts.infradead.org (moderated list:ARM/Ambarella SoC
        support)
Subject: Re: [PATCH 12/15] mtd: nand: add Ambarella nand support

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