[PATCH] mtd: nand: Add error log for marvell_nfc_end_cmd()

[PATCH] mtd: nand: Add error log for marvell_nfc_end_cmd()

[Wentao Liang]

The function marvell_nfc_hw_ecc_bch_read_chunk() calls the function
marvell_nfc_end_cmd(), but dose not check its return value. Since it
is incorrect to bail from the loop if the marvell_nfc_end_cmd() fails,
an error log is necessary to prevent silent failure.

Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/mtd/nand/raw/marvell_nand.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index 303b3016a070..1069a17dae9d 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -1351,16 +1351,20 @@ static void marvell_nfc_hw_ecc_bch_read_chunk(struct nand_chip *chip, int chunk,
 	 * Length is a multiple of 32 bytes, hence it is a multiple of 8 too.
 	 */
 	for (i = 0; i < data_len; i += FIFO_DEPTH * BCH_SEQ_READS) {
-		marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
-				    "RDDREQ while draining FIFO (data)");
+		err = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
+					  "RDDREQ while draining FIFO (data)");
+		if (err)
+			dev_err(nfc->dev, "Fail to confirm the NDSR.RDDREQ");
 		marvell_nfc_xfer_data_in_pio(nfc, data,
 					     FIFO_DEPTH * BCH_SEQ_READS);
 		data += FIFO_DEPTH * BCH_SEQ_READS;
 	}
 
 	for (i = 0; i < spare_len; i += FIFO_DEPTH * BCH_SEQ_READS) {
-		marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
-				    "RDDREQ while draining FIFO (OOB)");
+		err = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
+					  "RDDREQ while draining FIFO (OOB)");
+		if (err)
+			dev_err(nfc->dev, "Fail to confirm the NDSR.RDDREQ");
 		marvell_nfc_xfer_data_in_pio(nfc, spare,
 					     FIFO_DEPTH * BCH_SEQ_READS);
 		spare += FIFO_DEPTH * BCH_SEQ_READS;
-- 
2.42.0.windows.2


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

Re: [PATCH] mtd: nand: Add error log for marvell_nfc_end_cmd()

[kernel test robot]

Hi Wentao,

kernel test robot noticed the following build errors:

[auto build test ERROR on mtd/nand/next]
[also build test ERROR on linus/master v6.15-rc1 next-20250404]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wentao-Liang/mtd-nand-Add-error-log-for-marvell_nfc_end_cmd/20250407-101213
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next
patch link:    https://lore.kernel.org/r/20250407020917.1242-1-vulab%40iscas.ac.cn
patch subject: [PATCH] mtd: nand: Add error log for marvell_nfc_end_cmd()
config: arm64-randconfig-001-20250407 (https://download.01.org/0day-ci/archive/20250407/202504071221.XxGT6EaS-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 92c93f5286b9ff33f27ff694d2dc33da1c07afdd)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250407/202504071221.XxGT6EaS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504071221.XxGT6EaS-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/mtd/nand/raw/marvell_nand.c:89:
   In file included from include/linux/dmaengine.h:12:
   In file included from include/linux/scatterlist.h:8:
   In file included from include/linux/mm.h:2224:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/mtd/nand/raw/marvell_nand.c:1354:3: error: use of undeclared identifier 'err'
    1354 |                 err = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
         |                 ^
   drivers/mtd/nand/raw/marvell_nand.c:1356:7: error: use of undeclared identifier 'err'
    1356 |                 if (err)
         |                     ^
   drivers/mtd/nand/raw/marvell_nand.c:1364:3: error: use of undeclared identifier 'err'
    1364 |                 err = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
         |                 ^
   drivers/mtd/nand/raw/marvell_nand.c:1366:7: error: use of undeclared identifier 'err'
    1366 |                 if (err)
         |                     ^
   3 warnings and 4 errors generated.


vim +/err +1354 drivers/mtd/nand/raw/marvell_nand.c

  1302	
  1303	static void marvell_nfc_hw_ecc_bch_read_chunk(struct nand_chip *chip, int chunk,
  1304						      u8 *data, unsigned int data_len,
  1305						      u8 *spare, unsigned int spare_len,
  1306						      int page)
  1307	{
  1308		struct marvell_nand_chip *marvell_nand = to_marvell_nand(chip);
  1309		struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
  1310		const struct marvell_hw_ecc_layout *lt = to_marvell_nand(chip)->layout;
  1311		int i, ret;
  1312		struct marvell_nfc_op nfc_op = {
  1313			.ndcb[0] = NDCB0_CMD_TYPE(TYPE_READ) |
  1314				   NDCB0_ADDR_CYC(marvell_nand->addr_cyc) |
  1315				   NDCB0_LEN_OVRD,
  1316			.ndcb[1] = NDCB1_ADDRS_PAGE(page),
  1317			.ndcb[2] = NDCB2_ADDR5_PAGE(page),
  1318			.ndcb[3] = data_len + spare_len,
  1319		};
  1320	
  1321		ret = marvell_nfc_prepare_cmd(chip);
  1322		if (ret)
  1323			return;
  1324	
  1325		if (chunk == 0)
  1326			nfc_op.ndcb[0] |= NDCB0_DBC |
  1327					  NDCB0_CMD1(NAND_CMD_READ0) |
  1328					  NDCB0_CMD2(NAND_CMD_READSTART);
  1329	
  1330		/*
  1331		 * Trigger the monolithic read on the first chunk, then naked read on
  1332		 * intermediate chunks and finally a last naked read on the last chunk.
  1333		 */
  1334		if (chunk == 0)
  1335			nfc_op.ndcb[0] |= NDCB0_CMD_XTYPE(XTYPE_MONOLITHIC_RW);
  1336		else if (chunk < lt->nchunks - 1)
  1337			nfc_op.ndcb[0] |= NDCB0_CMD_XTYPE(XTYPE_NAKED_RW);
  1338		else
  1339			nfc_op.ndcb[0] |= NDCB0_CMD_XTYPE(XTYPE_LAST_NAKED_RW);
  1340	
  1341		marvell_nfc_send_cmd(chip, &nfc_op);
  1342	
  1343		/*
  1344		 * According to the datasheet, when reading from NDDB
  1345		 * with BCH enabled, after each 32 bytes reads, we
  1346		 * have to make sure that the NDSR.RDDREQ bit is set.
  1347		 *
  1348		 * Drain the FIFO, 8 32-bit reads at a time, and skip
  1349		 * the polling on the last read.
  1350		 *
  1351		 * Length is a multiple of 32 bytes, hence it is a multiple of 8 too.
  1352		 */
  1353		for (i = 0; i < data_len; i += FIFO_DEPTH * BCH_SEQ_READS) {
> 1354			err = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
  1355						  "RDDREQ while draining FIFO (data)");
  1356			if (err)
  1357				dev_err(nfc->dev, "Fail to confirm the NDSR.RDDREQ");
  1358			marvell_nfc_xfer_data_in_pio(nfc, data,
  1359						     FIFO_DEPTH * BCH_SEQ_READS);
  1360			data += FIFO_DEPTH * BCH_SEQ_READS;
  1361		}
  1362	
  1363		for (i = 0; i < spare_len; i += FIFO_DEPTH * BCH_SEQ_READS) {
  1364			err = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
  1365						  "RDDREQ while draining FIFO (OOB)");
  1366			if (err)
  1367				dev_err(nfc->dev, "Fail to confirm the NDSR.RDDREQ");
  1368			marvell_nfc_xfer_data_in_pio(nfc, spare,
  1369						     FIFO_DEPTH * BCH_SEQ_READS);
  1370			spare += FIFO_DEPTH * BCH_SEQ_READS;
  1371		}
  1372	}
  1373	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

Re: [PATCH] mtd: nand: Add error log for marvell_nfc_end_cmd()

[kernel test robot]

Hi Wentao,

kernel test robot noticed the following build errors:

[auto build test ERROR on mtd/nand/next]
[also build test ERROR on linus/master v6.15-rc1 next-20250407]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wentao-Liang/mtd-nand-Add-error-log-for-marvell_nfc_end_cmd/20250407-101213
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next
patch link:    https://lore.kernel.org/r/20250407020917.1242-1-vulab%40iscas.ac.cn
patch subject: [PATCH] mtd: nand: Add error log for marvell_nfc_end_cmd()
config: arm64-randconfig-003-20250407 (https://download.01.org/0day-ci/archive/20250407/202504071218.tZSv0K1o-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250407/202504071218.tZSv0K1o-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504071218.tZSv0K1o-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/mtd/nand/raw/marvell_nand.c: In function 'marvell_nfc_hw_ecc_bch_read_chunk':
>> drivers/mtd/nand/raw/marvell_nand.c:1354:3: error: 'err' undeclared (first use in this function)
      err = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
      ^~~
   drivers/mtd/nand/raw/marvell_nand.c:1354:3: note: each undeclared identifier is reported only once for each function it appears in


vim +/err +1354 drivers/mtd/nand/raw/marvell_nand.c

  1302	
  1303	static void marvell_nfc_hw_ecc_bch_read_chunk(struct nand_chip *chip, int chunk,
  1304						      u8 *data, unsigned int data_len,
  1305						      u8 *spare, unsigned int spare_len,
  1306						      int page)
  1307	{
  1308		struct marvell_nand_chip *marvell_nand = to_marvell_nand(chip);
  1309		struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
  1310		const struct marvell_hw_ecc_layout *lt = to_marvell_nand(chip)->layout;
  1311		int i, ret;
  1312		struct marvell_nfc_op nfc_op = {
  1313			.ndcb[0] = NDCB0_CMD_TYPE(TYPE_READ) |
  1314				   NDCB0_ADDR_CYC(marvell_nand->addr_cyc) |
  1315				   NDCB0_LEN_OVRD,
  1316			.ndcb[1] = NDCB1_ADDRS_PAGE(page),
  1317			.ndcb[2] = NDCB2_ADDR5_PAGE(page),
  1318			.ndcb[3] = data_len + spare_len,
  1319		};
  1320	
  1321		ret = marvell_nfc_prepare_cmd(chip);
  1322		if (ret)
  1323			return;
  1324	
  1325		if (chunk == 0)
  1326			nfc_op.ndcb[0] |= NDCB0_DBC |
  1327					  NDCB0_CMD1(NAND_CMD_READ0) |
  1328					  NDCB0_CMD2(NAND_CMD_READSTART);
  1329	
  1330		/*
  1331		 * Trigger the monolithic read on the first chunk, then naked read on
  1332		 * intermediate chunks and finally a last naked read on the last chunk.
  1333		 */
  1334		if (chunk == 0)
  1335			nfc_op.ndcb[0] |= NDCB0_CMD_XTYPE(XTYPE_MONOLITHIC_RW);
  1336		else if (chunk < lt->nchunks - 1)
  1337			nfc_op.ndcb[0] |= NDCB0_CMD_XTYPE(XTYPE_NAKED_RW);
  1338		else
  1339			nfc_op.ndcb[0] |= NDCB0_CMD_XTYPE(XTYPE_LAST_NAKED_RW);
  1340	
  1341		marvell_nfc_send_cmd(chip, &nfc_op);
  1342	
  1343		/*
  1344		 * According to the datasheet, when reading from NDDB
  1345		 * with BCH enabled, after each 32 bytes reads, we
  1346		 * have to make sure that the NDSR.RDDREQ bit is set.
  1347		 *
  1348		 * Drain the FIFO, 8 32-bit reads at a time, and skip
  1349		 * the polling on the last read.
  1350		 *
  1351		 * Length is a multiple of 32 bytes, hence it is a multiple of 8 too.
  1352		 */
  1353		for (i = 0; i < data_len; i += FIFO_DEPTH * BCH_SEQ_READS) {
> 1354			err = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
  1355						  "RDDREQ while draining FIFO (data)");
  1356			if (err)
  1357				dev_err(nfc->dev, "Fail to confirm the NDSR.RDDREQ");
  1358			marvell_nfc_xfer_data_in_pio(nfc, data,
  1359						     FIFO_DEPTH * BCH_SEQ_READS);
  1360			data += FIFO_DEPTH * BCH_SEQ_READS;
  1361		}
  1362	
  1363		for (i = 0; i < spare_len; i += FIFO_DEPTH * BCH_SEQ_READS) {
  1364			err = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
  1365						  "RDDREQ while draining FIFO (OOB)");
  1366			if (err)
  1367				dev_err(nfc->dev, "Fail to confirm the NDSR.RDDREQ");
  1368			marvell_nfc_xfer_data_in_pio(nfc, spare,
  1369						     FIFO_DEPTH * BCH_SEQ_READS);
  1370			spare += FIFO_DEPTH * BCH_SEQ_READS;
  1371		}
  1372	}
  1373	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

Re: [PATCH] mtd: nand: Add error log for marvell_nfc_end_cmd()

[Miquel Raynal]

Hello Liang,

On 07/04/2025 at 10:09:16 +08, Wentao Liang <vulab@iscas.ac.cn> wrote:

> The function marvell_nfc_hw_ecc_bch_read_chunk() calls the function
> marvell_nfc_end_cmd(), but dose not check its return value. Since it
> is incorrect to bail from the loop if the marvell_nfc_end_cmd() fails,
> an error log is necessary to prevent silent failure.

The failure will not be silent, *_read_chunk() is called in
marvell_nfc_hw_ecc_bch_read_page() only, where the validity of the data
will be checked. If you really want to make it visible *there* I believe
the correct approach is to change the caller prototype and forward the
error instead.

Thanks,
Miquèl

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