Re: [PATCH v3 05/40] mtd: rawnand: Create a new enumeration to describe properly ECC types
From: Boris Brezillon <boris.brezillon@collabora.com>
Date: 2019-10-12 09:14:08
On Thu, 19 Sep 2019 21:31:05 +0200 Miquel Raynal [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Now that the misleading mix between ECC engine type and OOB placement has been addressed, add a new enumeration to properly define ECC types (also called provider or mode). Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/raw/nand_base.c | 7 +++++++ include/linux/mtd/rawnand.h | 16 ++++++++++++++++ 2 files changed, 23 insertions(+)diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 9a05ebfc44d1..00a261284aad 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c@@ -4842,6 +4842,13 @@ static const char * const nand_ecc_modes[] = { [NAND_ECC_ON_DIE] = "on-die", }; +static const char * const nand_ecc_engine_providers[] = { + [NAND_NO_ECC_ENGINE] = "none", + [NAND_SOFT_ECC_ENGINE] = "soft", + [NAND_HW_ECC_ENGINE] = "hw", + [NAND_ON_DIE_ECC_ENGINE] = "on-die", +}; + static const char * const nand_ecc_engine_oob_placement[] = { [NAND_ECC_SYNDROME_OOB_PLACEMENT] = "hw_syndrome", [NAND_ECC_OOB_FIRST_PLACEMENT] = "hw_oob_first",diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index 1b462fb2ab77..23feab162bc2 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h@@ -93,6 +93,22 @@ enum nand_ecc_mode { NAND_ECC_ON_DIE, }; +/** + * enum nand_ecc_engine_type - NAND ECC engine type/provider + * @NAND_INVALID_ECC_ENGINE: Invalid value + * @NAND_NO_ECC_ENGINE: No ECC correction + * @NAND_SOFT_ECC_ENGINE: Software ECC correction + * @NAND_HW_ECC_ENGINE: Hardware (controller side) ECC correction + * @NAND_ON_DIE_ECC_ENGINE: Hardware (chip side) ECC correction + */ +enum nand_ecc_engine_type { + NAND_INVALID_ECC_ENGINE,
Same comment as for the NAND_ECC_INVALID addition: if you don't have an entry in nand_ecc_engine_providers for this INVALID case, it's probably better to define it to -1.
+ NAND_NO_ECC_ENGINE, + NAND_SOFT_ECC_ENGINE, + NAND_HW_ECC_ENGINE,
I'd rename that one into NAND_CONTROLLER_ECC_ENGINE, HW is a bit too vague.
+ NAND_ON_DIE_ECC_ENGINE,
I also find it clearer when the same prefix is used: NAND_ECC_ENGINE_INVALID = -1, NAND_ECC_ENGINE_NONE = 0, NAND_ECC_ENGINE_SOFT, NAND_ECC_ENGINE_CONTROLLER, NAND_ECC_ENGINE_ON_DIE, Looks good otherwise. Feel free to ignore my comments if you disagree. Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
+}; + /** * enum nand_ecc_engine_oob_placement - NAND ECC engine OOB placement * @NAND_ECC_DEFAULT_OOB_PLACEMENT: Standard layout, or not specified
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel