Skip to contentSkip to conversation

mtd: rawnand: qcom: Update code word value for raw read

PATCH Superseded Md Sadre Alam 2 participants 3 messages Sep 2, 2021 – Sep 7, 2021

Subsystems (3)

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v4 1/1 mtd: rawnand: qcom: Update code word value for raw read

From QPIC V2 onwards there is a separate register to read last code word "QPIC_NAND_READ_LOCATION_LAST_CW_n".

qcom_nandc_read_cw_raw() is used to read only one code word at a time. If we will configure number of code words to 1 in in QPIC_NAND_DEV0_CFG0 register then QPIC controller thinks its reading the last code word, since from QPIC V2 onwards we are having separate register to read the last code word, we have to configure "QPIC_NAND_READ_LOCATION_LAST_CW_n" register to fetch data from controller buffer to system memory.

Fixes 503ee5aa ("mtd: rawnand: qcom: update last code word register")

---
[V4]
* Added commit message
* Added changelog

In commit 503ee5aa added QPIC V2 support. Since QPIC V2 onwards there is separate register to get last CW data. If total number of CW configred is 1 then , QPIC controller treat this as last CW and software shhould copy data to memory via QPIC_NAND_READ_LOCATION_LAST_CW_n register instead of QPIC_NAND_READ_LOCATION_n. Since in raw read we are configuring total number of CW 1, this change fixes this if total number of CW 1 then make this as last CW. raw_cw = ecc->steps - 1; since ecc->steps holds total number of CWs.

 drivers/mtd/nand/raw/qcom_nandc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/mtd/nand/raw/qcom_nandc.c +6 -2
index ef0bade..04e6f7b 100644
@@ -1676,13 +1676,17 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	int data_size1, data_size2, oob_size1, oob_size2;
 	int ret, reg_off = FLASH_BUF_ACC, read_loc = 0;
+	int raw_cw = cw;
 
 	nand_read_page_op(chip, page, 0, NULL, 0);
 	host->use_ecc = false;
 
+	if (nandc->props->qpic_v2)
+		raw_cw = ecc->steps - 1;
+
 	clear_bam_transaction(nandc);
 	set_address(host, host->cw_size * cw, page);
-	update_rw_regs(host, 1, true, cw);
+	update_rw_regs(host, 1, true, raw_cw);
 	config_nand_page_read(chip);
 
 	data_size1 = mtd->writesize - host->cw_size * (ecc->steps - 1);
@@ -1711,7 +1715,7 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip,
 		nandc_set_read_loc(chip, cw, 3, read_loc, oob_size2, 1);
 	}
 
-	config_nand_cw_read(chip, false, cw);
+	config_nand_cw_read(chip, false, raw_cw);
 
 	read_data_dma(nandc, reg_off, data_buf, data_size1, 0);
 	reg_off += data_size1;
Signature · 1 line
2.7.4
#1 lore raw
Quoted 12 lines · Md Sadre Alam

From QPIC V2 onwards there is a separate register to read last code word "QPIC_NAND_READ_LOCATION_LAST_CW_n".

qcom_nandc_read_cw_raw() is used to read only one code word at a time. If we will configure number of code words to 1 in in QPIC_NAND_DEV0_CFG0 register then QPIC controller thinks its reading the last code word, since from QPIC V2 onwards we are having separate register to read the last code word, we have to configure "QPIC_NAND_READ_LOCATION_LAST_CW_n" register to fetch data from controller buffer to system memory.

Quoted 1 line
Fixes 503ee5aa ("mtd: rawnand: qcom: update last code word register")

Commit hash should be of 12 digits.

Quoted 7 lines

---
[V4]
* Added commit message
* Added changelog

Quoted 8 lines

In commit 503ee5aa added QPIC V2 support. Since QPIC V2 onwards there is separate register to get last CW data. If total number of CW configred is 1 then , QPIC controller treat this as last CW and software shhould copy data to memory via QPIC_NAND_READ_LOCATION_LAST_CW_n register instead of QPIC_NAND_READ_LOCATION_n. Since in raw read we are configuring total number of CW 1, this change fixes this if total number of CW 1 then make this as last CW. raw_cw = ecc->steps - 1; since ecc->steps holds total number of CWs.

This is not a changelog. Changelog should mention what has been changed between versions from v1 to v4. For example:

Changes in v4:

* Incorporated AAA comments from X
* Changed a local variable

Changes in v3:

* Incorporated BBB comments from X

Changes in v2:

* Incorporated CCC comments from X

Quoted 9 lines
 drivers/mtd/nand/raw/qcom_nandc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/mtd/nand/raw/qcom_nandc.c +0 -0
index ef0bade..04e6f7b 100644
@@ -1676,13 +1676,17 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	struct nand_ecc_ctrl *ecc = &chip->ecc;

drivers/mtd/nand/raw/qcom_nandc.c qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip, · line 1686

Quoted 10 lines
drivers/mtd/nand/raw/qcom_nandc.c +4 -0
 	int data_size1, data_size2, oob_size1, oob_size2;
 	int ret, reg_off = FLASH_BUF_ACC, read_loc = 0;
+	int raw_cw = cw;
 
 	nand_read_page_op(chip, page, 0, NULL, 0);
 	host->use_ecc = false;
 
+	if (nandc->props->qpic_v2)
+		raw_cw = ecc->steps - 1;
+

Sorry, I don't understand how it can work. For reading a codeword like 0, you are reading the last codeword here if the controller is v2. And you'll continue to read the last codeword only for any codeword requested :/

Thanks,
Mani

Quoted 19 lines
drivers/mtd/nand/raw/qcom_nandc.c +2 -2
 	clear_bam_transaction(nandc);
 	set_address(host, host->cw_size * cw, page);
-	update_rw_regs(host, 1, true, cw);
+	update_rw_regs(host, 1, true, raw_cw);
 	config_nand_page_read(chip);
 
 	data_size1 = mtd->writesize - host->cw_size * (ecc->steps - 1);
@@ -1711,7 +1715,7 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip,
 		nandc_set_read_loc(chip, cw, 3, read_loc, oob_size2, 1);
 	}
 
-	config_nand_cw_read(chip, false, cw);
+	config_nand_cw_read(chip, false, raw_cw);
 
 	read_data_dma(nandc, reg_off, data_buf, data_size1, 0);
 	reg_off += data_size1;
Signature · 1 line
2.7.4
#2 lore raw
Quoted 13 lines · Manivannan Sadhasivam
Quoted 12 lines · Md Sadre Alam

From QPIC V2 onwards there is a separate register to read last code word "QPIC_NAND_READ_LOCATION_LAST_CW_n".

qcom_nandc_read_cw_raw() is used to read only one code word at a time. If we will configure number of code words to 1 in in QPIC_NAND_DEV0_CFG0 register then QPIC controller thinks its reading the last code word, since from QPIC V2 onwards we are having separate register to read the last code word, we have to configure "QPIC_NAND_READ_LOCATION_LAST_CW_n" register to fetch data from controller buffer to system memory.

Quoted 3 lines
Quoted 1 line
Fixes 503ee5aa ("mtd: rawnand: qcom: update last code word register")

Commit hash should be of 12 digits.

Updated in patch V5

Quoted 30 lines
Quoted 8 lines

---
[V4]
* Added commit message
* Added changelog

Quoted 13 lines

In commit 503ee5aa added QPIC V2 support. Since QPIC V2 onwards there is separate register to get last CW data. If total number of CW
configred is 1
then , QPIC controller treat this as last CW and software shhould
copy data to memory
via QPIC_NAND_READ_LOCATION_LAST_CW_n register instead of
QPIC_NAND_READ_LOCATION_n.
Since in raw read we are configuring total number of CW 1, this
change fixes
this if total number of CW 1 then make this as last CW. raw_cw =
ecc->steps - 1;
since ecc->steps holds total number of CWs.

This is not a changelog. Changelog should mention what has been changed between versions from v1 to v4. For example:

Changes in v4:

* Incorporated AAA comments from X
* Changed a local variable

Quoted 7 lines

Changes in v3:

* Incorporated BBB comments from X

Changes in v2:

* Incorporated CCC comments from X

Updated in patch V5

Quoted 18 lines
Quoted 18 lines
 drivers/mtd/nand/raw/qcom_nandc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/mtd/nand/raw/qcom_nandc.c +1 -0
index ef0bade..04e6f7b 100644
@@ -1676,13 +1676,17 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, 
 struct nand_chip *chip,
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	int data_size1, data_size2, oob_size1, oob_size2;
 	int ret, reg_off = FLASH_BUF_ACC, read_loc = 0;
+	int raw_cw = cw;
 
 	nand_read_page_op(chip, page, 0, NULL, 0);
 	host->use_ecc = false;
 
Quoted 8 lines

drivers/mtd/nand/raw/qcom_nandc.c qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip, · line 1686

Quoted 3 lines
drivers/mtd/nand/raw/qcom_nandc.c +3 -0
+	if (nandc->props->qpic_v2)
+		raw_cw = ecc->steps - 1;
+

Sorry, I don't understand how it can work. For reading a codeword like 0, you are reading the last codeword here if the controller is v2. And you'll continue to read the last codeword only for any codeword requested :/

This case is only applicable when we will configure number of code
words to 1.
These registers "QPIC_NAND_READ_LOCATION_LAST_CW_n" &
"QPIC_NAND_READ_LOCATION_n"
are only used to copy data into memory i.e our local buffer. So this
last CW interpretation from
QPIC controller to memory not for Flash to QPIC controller. If number of CW configured to 1 means QPIC will treat this a last CW ,
so copy the data from QPIC to local memory
buffer , we have to use QPIC_NAND_READ_LOCATION_LAST_CW_n register
instead of QPIC_NAND_READ_LOCATION_n regsiter,
Since there is separate register from QPIC V2 onwards to read get last
CW "QPIC_NAND_READ_LOCATION_LAST_CW_n".

Quoted 24 lines

Thanks,
Mani

Quoted 20 lines
drivers/mtd/nand/raw/qcom_nandc.c +1 -1
 	clear_bam_transaction(nandc);
 	set_address(host, host->cw_size * cw, page);
-	update_rw_regs(host, 1, true, cw);
+	update_rw_regs(host, 1, true, raw_cw);
 	config_nand_page_read(chip);
 
drivers/mtd/nand/raw/qcom_nandc.c +1 -1
 	data_size1 = mtd->writesize - host->cw_size * (ecc->steps - 1);
@@ -1711,7 +1715,7 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, 
 struct nand_chip *chip,
 		nandc_set_read_loc(chip, cw, 3, read_loc, oob_size2, 1);
 	}
 
-	config_nand_cw_read(chip, false, cw);
+	config_nand_cw_read(chip, false, raw_cw);
 
 	read_data_dma(nandc, reg_off, data_buf, data_size1, 0);

reg_off += data_size1;
--
2.7.4