[PATCH] mtd: nand: ecc-hamming: Clarify the logic around rp17

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>,
	<linux-mtd@lists.infradead.org>
Cc: kernel test robot <lkp@intel.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Subject: [PATCH] mtd: nand: ecc-hamming: Clarify the logic around rp17
Date: Thu, 29 Oct 2020 09:38:47 +0100	[thread overview]
Message-ID: <20201029083847.21128-1-miquel.raynal@bootlin.com> (raw)

This code has been written in 2008 and is fine, but in order to keep
robots happy, I think it's time to change a little bit this code just
to clarify the different possible values of eccsize_mult. Indeed, this
variable may only take the value 1 or 2 because step_size, in the case
of the software Hamming ECC engine may only be 256 or 512. Depending
on the value of eccsize_mult, an extra rp17 variable is set, or not
and triggers the following warning:

     smatch warnings:
     ecc_sw_hamming_calculate() error: uninitialized symbol 'rp17'.

As highlighted by Dan Carpenter, if the only possible values for
eccsize_mult are 1 and 2, then the code is fine, but "it's hard to
tell just from looking".

So instead of shifting step_size, let's use a ternary condition to
assign to eccsize_mult the only two possible values and clarify the
driver's logic.

Now that the situation is clarified for humans, set rp17 to 0 in an
else block to keep robots silent as well.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc-sw-hamming.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/ecc-sw-hamming.c b/drivers/mtd/nand/ecc-sw-hamming.c
index 334e619b35c9..9a1aa9005970 100644
--- a/drivers/mtd/nand/ecc-sw-hamming.c
+++ b/drivers/mtd/nand/ecc-sw-hamming.c
@@ -116,7 +116,7 @@ int ecc_sw_hamming_calculate(const unsigned char *buf, unsigned int step_size,
 			     unsigned char *code, bool sm_order)
 {
 	const u32 *bp = (uint32_t *)buf;
-	const u32 eccsize_mult = step_size >> 8;
+	const u32 eccsize_mult = (step_size == 256) ? 1 : 2;
 	/* current value in buffer */
 	u32 cur;
 	/* rp0..rp17 are the various accumulated parities (per byte) */
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
