| 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: <20240701095906.2bc4a0d2@xps-13>
Date: Mon, 1 Jul 2024 09:59:06 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Maxim Anisimov <maxim.anisimov.ua@...il.com>
Cc: Richard Weinberger <richard@....at>, Vignesh Raghavendra
<vigneshr@...com>, Martin Kurbanov <mmkurbanov@...utedevices.com>, Michael
Walle <michael@...le.cc>, Mark Brown <broonie@...nel.org>, "Chia-Lin Kao
(AceLan)" <acelan.kao@...onical.com>, linux-kernel@...r.kernel.org,
linux-mtd@...ts.infradead.org
Subject: Re: [PATCH] mtd: spinand: Add support for HeYangTek HYF1GQ4UDACAE
Hi Maxim,
maxim.anisimov.ua@...il.com wrote on Mon, 24 Jun 2024 09:12:17 +0300:
> Add Support HeYangTek HYF1GQ4UDACAE SPI NAND.
>
> Datasheet Link:
> - https://www.heyangtek.cn/previewfile.jsp?file=ABUIABA9GAAgwsvRnwYo-eDpsgc
Thanks for the patch! Few comments below.
> Signed-off-by: Maxim Anisimov <maxim.anisimov.ua@...il.com>
> ---
> drivers/mtd/nand/spi/Makefile | 4 +-
> drivers/mtd/nand/spi/core.c | 1 +
> drivers/mtd/nand/spi/heyangtek.c | 112 +++++++++++++++++++++++++++++++
> include/linux/mtd/spinand.h | 1 +
> 4 files changed, 116 insertions(+), 2 deletions(-)
> create mode 100644 drivers/mtd/nand/spi/heyangtek.c
>
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> index 19cc77288ebb..69d95fbdd0ce 100644
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> -spinand-objs := core.o alliancememory.o ato.o esmt.o foresee.o gigadevice.o macronix.o
> -spinand-objs += micron.o paragon.o toshiba.o winbond.o xtx.o
> +spinand-objs := core.o alliancememory.o ato.o esmt.o foresee.o gigadevice.o heyangtek.o
> +spinand-objs += macronix.o micron.o paragon.o toshiba.o winbond.o xtx.o
> obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index e0b6715e5dfe..45795e5f1e49 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -942,6 +942,7 @@ static const struct spinand_manufacturer *spinand_manufacturers[] = {
> &esmt_c8_spinand_manufacturer,
> &foresee_spinand_manufacturer,
> &gigadevice_spinand_manufacturer,
> + &heyangtek_spinand_manufacturer,
> ¯onix_spinand_manufacturer,
> µn_spinand_manufacturer,
> ¶gon_spinand_manufacturer,
> diff --git a/drivers/mtd/nand/spi/heyangtek.c b/drivers/mtd/nand/spi/heyangtek.c
> new file mode 100644
> index 000000000000..d4a5dbca40fb
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/heyangtek.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Author:
> + * Andrey Zolotarev <andrey.zolotarev@...netic.com> - the main driver logic
> + * Maxim Anisimov <maxim.anisimov.ua@...il.com> - adaptation to mainline linux kernel
> + *
> + * Based on:
> + * https://github.com/keenetic/kernel-49/commit/bacade569fb12bc0ad31ba09bca9b890118fbca7
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mtd/spinand.h>
> +
> +#define SPINAND_MFR_HEYANGTEK 0xC9
> +
> +#define STATUS_ECC_LIMIT_BITFLIPS (3 << 4)
> +
> +static SPINAND_OP_VARIANTS(read_cache_variants,
> + SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(write_cache_variants,
> + SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
> + SPINAND_PROG_LOAD(true, 0, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(update_cache_variants,
> + SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
> + SPINAND_PROG_LOAD(false, 0, NULL, 0));
> +
> +static int hyfxgq4uda_ooblayout_ecc(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *region)
> +{
> + if (section > 3)
> + return -ERANGE;
> +
> + region->offset = section * 16 + 8;
> + region->length = 8;
This is: 8-15, 24-31, 40-47, 56-62
> +
> + return 0;
> +}
> +
> +static int hyfxgq4uda_ooblayout_free(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *region)
> +{
> + if (section > 3)
> + return -ERANGE;
> +
> + /* ECC-protected user meta-data */
> + region->offset = section * 16 + 4;
> + region->length = 4;
This is: 4-7, 20-23, 32-35, 48-51
So what about 2-4, 16-19, 36-39, 52-55, 63-64 ?
> +
> + return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops hyfxgq4uda_ooblayout = {
> + .ecc = hyfxgq4uda_ooblayout_ecc,
> + .free = hyfxgq4uda_ooblayout_free,
> +};
> +
> +static int hyfxgq4uda_ecc_get_status(struct spinand_device *spinand,
> + u8 status)
> +{
> + struct nand_device *nand = spinand_to_nand(spinand);
> +
> + switch (status & STATUS_ECC_MASK) {
> + case STATUS_ECC_NO_BITFLIPS:
> + return 0;
> +
> + case STATUS_ECC_UNCOR_ERROR:
> + return -EBADMSG;
> +
> + case STATUS_ECC_HAS_BITFLIPS:
> + return nanddev_get_ecc_conf(nand)->strength >> 1;
Maybe an explanation of this line is needed. Is this just guessing or
is this defined in the datasheet?
Also please do not use shifts when you want to divide. Just use / 2
which is easier to understand. Compilers know how to optimize that.
> +
> + case STATUS_ECC_LIMIT_BITFLIPS:
> + return nanddev_get_ecc_conf(nand)->strength;
> +
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct spinand_info heyangtek_spinand_table[] = {
> + SPINAND_INFO("HYF1GQ4UDACAE",
> + SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x21),
> + NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1),
> + NAND_ECCREQ(4, 512),
> + SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> + &write_cache_variants,
> + &update_cache_variants),
> + SPINAND_HAS_QE_BIT,
> + SPINAND_ECCINFO(&hyfxgq4uda_ooblayout,
> + hyfxgq4uda_ecc_get_status)),
> +};
> +
> +static const struct spinand_manufacturer_ops heyangtek_spinand_manuf_ops = {
> +};
> +
> +const struct spinand_manufacturer heyangtek_spinand_manufacturer = {
> + .id = SPINAND_MFR_HEYANGTEK,
> + .name = "HeYangTek",
> + .chips = heyangtek_spinand_table,
> + .nchips = ARRAY_SIZE(heyangtek_spinand_table),
> + .ops = &heyangtek_spinand_manuf_ops,
> +};
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> index 5c19ead60499..06ee35a27e3b 100644
> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -265,6 +265,7 @@ extern const struct spinand_manufacturer ato_spinand_manufacturer;
> extern const struct spinand_manufacturer esmt_c8_spinand_manufacturer;
> extern const struct spinand_manufacturer foresee_spinand_manufacturer;
> extern const struct spinand_manufacturer gigadevice_spinand_manufacturer;
> +extern const struct spinand_manufacturer heyangtek_spinand_manufacturer;
> extern const struct spinand_manufacturer macronix_spinand_manufacturer;
> extern const struct spinand_manufacturer micron_spinand_manufacturer;
> extern const struct spinand_manufacturer paragon_spinand_manufacturer;
Thanks,
Miquèl
Powered by blists - more mailing lists