Hi Boris,

quoted

+/**
+ * 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.

quoted

+	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.

quoted

+	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.

Same prefix it is!

However, I don't want to start as -1 as otherwise using a for-loop to
loop over each ECC engine would be less straightforward.

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel