| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20180526104629.74315561@xps13>
Date: Sat, 26 May 2018 10:46:29 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Abhishek Sahu <absahu@...eaurora.org>
Cc: Boris Brezillon <boris.brezillon@...tlin.com>,
David Woodhouse <dwmw2@...radead.org>,
Brian Norris <computersforpeace@...il.com>,
Marek Vasut <marek.vasut@...il.com>,
Richard Weinberger <richard@....at>,
Cyrille Pitchen <cyrille.pitchen@...ev4u.fr>,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mtd@...ts.infradead.org, Andy Gross <andy.gross@...aro.org>,
Archit Taneja <architt@...eaurora.org>
Subject: Re: [PATCH v3 13/16] mtd: rawnand: qcom: minor code reorganization
for bad block check
Hi Abhishek,
On Fri, 25 May 2018 17:51:41 +0530, Abhishek Sahu
<absahu@...eaurora.org> wrote:
> The QCOM NAND controller layout is such that, the bad block byte
> offset for last codeword will come to first byte in spare area.
"is the first spare byte"?
> Currently, the raw read for last codeword is being done with
> copy_last_cw function. It does following 2 things:
"It does the following:"
>
> 1. Read the last codeword bytes from NAND chip to NAND
> controller internal HW buffer.
> 2. Copy all these bytes from HW buffer to actual buffer.
>
> For bad block check, maximum two bytes are required so instead of
> copying the complete bytes in step 2, only those bbm_size bytes
> can be copied.
>
> This patch does minor code reorganization for the same. After
> this, copy_last_cw function won’t be required.
"This patch does minor code reorganization to just retrieve these two
bytes when checking for bad blocks, allowing to remove
copy_last_cw() now useless."
>
> Signed-off-by: Abhishek Sahu <absahu@...eaurora.org>
> ---
> * Changes from v2:
> 1. Changed commit message and comments slightly
>
> * Changes from v1:
> NEW CHANGE
>
> drivers/mtd/nand/raw/qcom_nandc.c | 66 +++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index d693b5f..f72bc8a 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -1769,41 +1769,6 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
> return parse_read_errors(host, data_buf_start, oob_buf_start);
> }
>
> -/*
> - * a helper that copies the last step/codeword of a page (containing free oob)
> - * into our local buffer
> - */
> -static int copy_last_cw(struct qcom_nand_host *host, int page)
> -{
> - struct nand_chip *chip = &host->chip;
> - struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
> - struct nand_ecc_ctrl *ecc = &chip->ecc;
> - int size;
> - int ret;
> -
> - clear_read_regs(nandc);
> -
> - size = host->use_ecc ? host->cw_data : host->cw_size;
> -
> - /* prepare a clean read buffer */
> - memset(nandc->data_buffer, 0xff, size);
> -
> - set_address(host, host->cw_size * (ecc->steps - 1), page);
> - update_rw_regs(host, 1, true);
> -
> - config_nand_single_cw_page_read(nandc);
> -
> - read_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer, size, 0);
> -
> - ret = submit_descs(nandc);
> - if (ret)
> - dev_err(nandc->dev, "failed to copy last codeword\n");
> -
> - free_descs(nandc);
> -
> - return ret;
> -}
> -
> /* implements ecc->read_page() */
> static int qcom_nandc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> uint8_t *buf, int oob_required, int page)
> @@ -2118,6 +2083,7 @@ static int qcom_nandc_block_bad(struct mtd_info *mtd, loff_t ofs)
> struct nand_ecc_ctrl *ecc = &chip->ecc;
> int page, ret, bbpos, bad = 0;
> u32 flash_status;
> + u8 *bbm_bytes_buf = chip->data_buf;
>
> page = (int)(ofs >> chip->page_shift) & chip->pagemask;
>
> @@ -2128,11 +2094,31 @@ static int qcom_nandc_block_bad(struct mtd_info *mtd, loff_t ofs)
> * that contains the BBM
> */
> host->use_ecc = false;
> + bbpos = mtd->writesize - host->cw_size * (ecc->steps - 1);
Are we sure there is no layout with only 1 step?
>
> clear_bam_transaction(nandc);
> - ret = copy_last_cw(host, page);
> - if (ret)
> + clear_read_regs(nandc);
> +
> + set_address(host, host->cw_size * (ecc->steps - 1), page);
> + update_rw_regs(host, 1, true);
> +
> + /*
> + * The last codeword data will be copied from NAND device to NAND
> + * controller internal HW buffer. Copy only required BBM size bytes
> + * from this HW buffer to bbm_bytes_buf which is present at
> + * bbpos offset.
> + */
> + nandc_set_read_loc(nandc, 0, bbpos, host->bbm_size, 1);
> + config_nand_single_cw_page_read(nandc);
> + read_data_dma(nandc, FLASH_BUF_ACC + bbpos, bbm_bytes_buf,
> + host->bbm_size, 0);
> +
> + ret = submit_descs(nandc);
> + free_descs(nandc);
> + if (ret) {
> + dev_err(nandc->dev, "failed to copy bad block bytes\n");
> goto err;
> + }
>
> flash_status = le32_to_cpu(nandc->reg_read_buf[0]);
>
> @@ -2141,12 +2127,10 @@ static int qcom_nandc_block_bad(struct mtd_info *mtd, loff_t ofs)
> goto err;
> }
>
> - bbpos = mtd->writesize - host->cw_size * (ecc->steps - 1);
> -
> - bad = nandc->data_buffer[bbpos] != 0xff;
> + bad = bbm_bytes_buf[0] != 0xff;
This is suspect as it still points to the beginning of the data buffer.
Can you please check you did not meant bbm_bytes_buf[bbpos]?
>
> if (chip->options & NAND_BUSWIDTH_16)
> - bad = bad || (nandc->data_buffer[bbpos + 1] != 0xff);
> + bad = bad || (bbm_bytes_buf[1] != 0xff);
> err:
> return bad;
> }
Thanks,
Miquèl
Powered by blists - more mailing lists