2023-08-05 19:01:46

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 00/10] mtd: rawnand: qcom: Bunch of fixes and cleanups

Hi Miquel,

This series has fixes for the smatch warnings reported by Kbuild bot [1]
and also several cleanup patches based on my code observation.

I've only compile tested this series. So let's wait for Sadre/Sricharan to
give a tested-by tag to make sure I didn't mess up anything.

- Mani

[1] https://lore.kernel.org/all/[email protected]/

Manivannan Sadhasivam (10):
mtd: rawnand: qcom: Remove superfluous initialization of "ret"
mtd: rawnand: qcom: Rename variables in qcom_op_cmd_mapping()
mtd: rawnand: qcom: Handle unsupported opcode in qcom_op_cmd_mapping()
mtd: rawnand: qcom: Fix the opcode check in qcom_check_op()
mtd: rawnand: qcom: Use EOPNOTSUPP instead of ENOTSUPP
mtd: rawnand: qcom: Wrap qcom_nand_exec_op() to 80 columns
mtd: rawnand: qcom: Unmap sg_list and free desc within submic_descs()
mtd: rawnand: qcom: Simplify the call to nand_prog_page_end_op()
mtd: rawnand: qcom: Do not override the error no of submit_descs()
mtd: rawnand: qcom: Sort includes alphabetically

drivers/mtd/nand/raw/qcom_nandc.c | 192 ++++++++++++++----------------
1 file changed, 91 insertions(+), 101 deletions(-)

--
2.25.1



2023-08-05 19:13:11

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 01/10] mtd: rawnand: qcom: Remove superfluous initialization of "ret"

In all the cases, "ret" variable is assigned a value before returning it.
So there is no need to explicitly initialize it with 0.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/mtd/nand/raw/qcom_nandc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index b485d8517fce..b6751fb17587 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1554,7 +1554,7 @@ check_for_erased_page(struct qcom_nand_host *host, u8 *data_buf,
struct mtd_info *mtd = nand_to_mtd(chip);
struct nand_ecc_ctrl *ecc = &chip->ecc;
u8 *cw_data_buf, *cw_oob_buf;
- int cw, data_size, oob_size, ret = 0;
+ int cw, data_size, oob_size, ret;

if (!data_buf)
data_buf = nand_get_data_buf(chip);
@@ -2684,7 +2684,7 @@ static int qcom_read_status_exec(struct nand_chip *chip,
const struct nand_op_instr *instr = NULL;
unsigned int op_id = 0;
unsigned int len = 0;
- int ret = 0, num_cw, i;
+ int ret, num_cw, i;
u32 flash_status;

host->status = NAND_STATUS_READY | NAND_STATUS_WP;
@@ -2747,7 +2747,7 @@ static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subo
const struct nand_op_instr *instr = NULL;
unsigned int op_id = 0;
unsigned int len = 0;
- int ret = 0;
+ int ret;

qcom_parse_instructions(chip, subop, &q_op);

@@ -2795,7 +2795,7 @@ static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_sub
struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
struct qcom_nand_host *host = to_qcom_nand_host(chip);
struct qcom_op q_op = {};
- int ret = 0;
+ int ret;

qcom_parse_instructions(chip, subop, &q_op);

@@ -2841,7 +2841,7 @@ static int qcom_param_page_type_exec(struct nand_chip *chip, const struct nand_
const struct nand_op_instr *instr = NULL;
unsigned int op_id = 0;
unsigned int len = 0;
- int ret = 0;
+ int ret;

qcom_parse_instructions(chip, subop, &q_op);

@@ -2935,7 +2935,7 @@ static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_su
struct qcom_nand_host *host = to_qcom_nand_host(chip);
struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
struct qcom_op q_op = {};
- int ret = 0;
+ int ret;

qcom_parse_instructions(chip, subop, &q_op);

--
2.25.1


2023-08-05 19:14:58

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 06/10] mtd: rawnand: qcom: Wrap qcom_nand_exec_op() to 80 columns

Both the function arguments and the definition could be wrapped to 80
columns to save line space.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/mtd/nand/raw/qcom_nandc.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index a7a9421ef003..4f38579ae03e 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -3051,14 +3051,12 @@ static int qcom_check_op(struct nand_chip *chip,
}

static int qcom_nand_exec_op(struct nand_chip *chip,
- const struct nand_operation *op,
- bool check_only)
+ const struct nand_operation *op, bool check_only)
{
if (check_only)
return qcom_check_op(chip, op);

- return nand_op_parser_exec_op(chip, &qcom_op_parser,
- op, check_only);
+ return nand_op_parser_exec_op(chip, &qcom_op_parser, op, check_only);
}

static const struct nand_controller_ops qcom_nandc_ops = {
--
2.25.1


2023-08-05 19:15:05

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 10/10] mtd: rawnand: qcom: Sort includes alphabetically

Sort includes in alphabetical order.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/mtd/nand/raw/qcom_nandc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 0fbc6d1a558c..d4ba0d04c970 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2,19 +2,19 @@
/*
* Copyright (c) 2016, The Linux Foundation. All rights reserved.
*/
-#include <linux/clk.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
#include <linux/bitops.h>
-#include <linux/dma/qcom_adm.h>
-#include <linux/dma-mapping.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/dma/qcom_adm.h>
+#include <linux/dma/qcom_bam_dma.h>
#include <linux/module.h>
-#include <linux/mtd/rawnand.h>
#include <linux/mtd/partitions.h>
+#include <linux/mtd/rawnand.h>
#include <linux/of.h>
-#include <linux/delay.h>
-#include <linux/dma/qcom_bam_dma.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>

/* NANDc reg offsets */
#define NAND_FLASH_CMD 0x00
--
2.25.1


2023-08-05 19:38:59

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 09/10] mtd: rawnand: qcom: Do not override the error no of submit_descs()

Just use the error no returned by submit_descs() instead of overriding it
with -EIO.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/mtd/nand/raw/qcom_nandc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 6b81781aa3ad..0fbc6d1a558c 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2140,7 +2140,7 @@ static int qcom_nandc_write_oob(struct nand_chip *chip, int page)
ret = submit_descs(nandc);
if (ret) {
dev_err(nandc->dev, "failure to write oob\n");
- return -EIO;
+ return ret;
}

return nand_prog_page_end_op(chip);
@@ -2216,7 +2216,7 @@ static int qcom_nandc_block_markbad(struct nand_chip *chip, loff_t ofs)
ret = submit_descs(nandc);
if (ret) {
dev_err(nandc->dev, "failure to update BBM\n");
- return -EIO;
+ return ret;
}

return nand_prog_page_end_op(chip);
--
2.25.1


2023-08-06 15:20:52

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH 00/10] mtd: rawnand: qcom: Bunch of fixes and cleanups

Hi Manivannan,

[email protected] wrote on Sat, 5 Aug 2023 23:11:36
+0530:

> Hi Miquel,
>
> This series has fixes for the smatch warnings reported by Kbuild bot [1]
> and also several cleanup patches based on my code observation.
>
> I've only compile tested this series. So let's wait for Sadre/Sricharan to
> give a tested-by tag to make sure I didn't mess up anything.

I reviewed all the patches, they look good to me. I'm waiting for the
tests. Please provide the output of nandbiterrs -i.

Thanks,
Miquèl

2023-08-07 15:33:34

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH 00/10] mtd: rawnand: qcom: Bunch of fixes and cleanups

On Sun, Aug 06, 2023 at 03:53:09PM +0200, Miquel Raynal wrote:
> Hi Manivannan,
>
> [email protected] wrote on Sat, 5 Aug 2023 23:11:36
> +0530:
>
> > Hi Miquel,
> >
> > This series has fixes for the smatch warnings reported by Kbuild bot [1]
> > and also several cleanup patches based on my code observation.
> >
> > I've only compile tested this series. So let's wait for Sadre/Sricharan to
> > give a tested-by tag to make sure I didn't mess up anything.
>
> I reviewed all the patches, they look good to me. I'm waiting for the
> tests. Please provide the output of nandbiterrs -i.
>

I tested the series on Qcom SDX55 based dev board, but it was broken already due
to commit <3fc92384b654> ("mtd: rawnand: qcom: Implement exec_op()").

I'm seeing a string of this error while the partitions were being enumerated:
qcom-nandc 1b30000.nand-controller: failed to copy last codeword

It just goes on...

Sadre, on what platform did you test 3fc92384b654? From a quick look, the BAM
DMA transactions are timing out.

- Mani

> Thanks,
> Miquèl

--
மணிவண்ணன் சதாசிவம்

2023-08-20 11:38:01

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH 06/10] mtd: rawnand: qcom: Wrap qcom_nand_exec_op() to 80 columns

On Sat, 2023-08-05 at 17:41:42 UTC, Manivannan Sadhasivam wrote:
> Both the function arguments and the definition could be wrapped to 80
> columns to save line space.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel