| 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: <f08ada1166fe5446ea0f6d679eb3cdae@codeaurora.org>
Date: Wed, 16 Aug 2017 13:04:51 +0530
From: Abhishek Sahu <absahu@...eaurora.org>
To: Archit Taneja <architt@...eaurora.org>
Cc: boris.brezillon@...e-electrons.com, dwmw2@...radead.org,
computersforpeace@...il.com, marek.vasut@...il.com, richard@....at,
cyrille.pitchen@...ev4u.fr, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org,
devicetree@...r.kernel.org, andy.gross@...aro.org,
sricharan@...eaurora.org
Subject: Re: [PATCH v4 09/20] mtd: nand: qcom: support for read location
registers
On 2017-08-16 10:04, Archit Taneja wrote:
> On 08/11/2017 05:09 PM, Abhishek Sahu wrote:
>> In EBI2, all codeword data will be read in FLASH_BUF_ACC buffer
>> and ADM will copy the data from source (FLASH_BUF_ACC) to
>> destination (memory for data read).
>>
>> In QPIC, there is no FLASH_BUF_ACC and all the codeword data will
>> held in QPIC BAM FIFO buffers. It provides multiple READ_LOCATION
>> registers which will be used for copying the data from FIFO to
>> memory. The READ_LOCATION register will be used to read a
>> specific amount of data from a specific offset within the flash
>> buffer. It supports sequential offset requests. Each request is
>> composed of the following fields:
>>
>> a. Offset within the flash buffer from which data should be
>> read
>> b. Amount of data to be read
>> c. Flag bit specifying the last read request from the flash
>> buffer. Following the last read request the NANDc refers to the
>> buffer as empty.
>>
>> Signed-off-by: Abhishek Sahu <absahu@...eaurora.org>
>> ---
>> drivers/mtd/nand/qcom_nandc.c | 64
>> +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 64 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/qcom_nandc.c
>> b/drivers/mtd/nand/qcom_nandc.c
>> index d9c8a6b..b452cfb 100644
>> --- a/drivers/mtd/nand/qcom_nandc.c
>> +++ b/drivers/mtd/nand/qcom_nandc.c
>> @@ -53,6 +53,8 @@
>> #define NAND_VERSION 0xf08
>> #define NAND_READ_LOCATION_0 0xf20
>> #define NAND_READ_LOCATION_1 0xf24
>> +#define NAND_READ_LOCATION_2 0xf28
>> +#define NAND_READ_LOCATION_3 0xf2c
>> /* dummy register offsets, used by write_reg_dma */
>> #define NAND_DEV_CMD1_RESTORE 0xdead
>> @@ -135,6 +137,11 @@
>> #define ERASED_PAGE (PAGE_ALL_ERASED | PAGE_ERASED)
>> #define ERASED_CW (CODEWORD_ALL_ERASED | CODEWORD_ERASED)
>> +/* NAND_READ_LOCATION_n bits */
>> +#define READ_LOCATION_OFFSET 0
>> +#define READ_LOCATION_SIZE 16
>> +#define READ_LOCATION_LAST 31
>> +
>> /* Version Mask */
>> #define NAND_VERSION_MAJOR_MASK 0xf0000000
>> #define NAND_VERSION_MAJOR_SHIFT 28
>> @@ -177,6 +184,12 @@
>> #define ECC_BCH_4BIT BIT(2)
>> #define ECC_BCH_8BIT BIT(3)
>> +#define nandc_set_readl(nandc, reg, offset, size, is_last) \
>
> Minor nit, readl makes one think it's 'read long'. If it isn't too
> much effort, could you s/nandc_set_readl/nandc_set_read_loc ?
>
Thanks Archit.
Yes that would be better. I will change the name.
>> +nandc_set_reg(nandc, NAND_READ_LOCATION_##reg, \
>> + ((offset) << READ_LOCATION_OFFSET) | \
>> + ((size) << READ_LOCATION_SIZE) | \
>> + ((is_last) << READ_LOCATION_LAST))
>> +
>> #define QPIC_PER_CW_CMD_SGL 32
>> #define QPIC_PER_CW_DATA_SGL 8
>> @@ -260,6 +273,11 @@ struct nandc_regs {
>> __le32 orig_vld;
>> __le32 ecc_buf_cfg;
>> + __le32 read_location0;
>> + __le32 read_location1;
>> + __le32 read_location2;
>> + __le32 read_location3;
>> +
>> };
>> /*
>> @@ -516,6 +534,14 @@ static __le32 *offset_to_nandc_reg(struct
>> nandc_regs *regs, int offset)
>> return ®s->orig_vld;
>> case NAND_EBI2_ECC_BUF_CFG:
>> return ®s->ecc_buf_cfg;
>> + case NAND_READ_LOCATION_0:
>> + return ®s->read_location0;
>> + case NAND_READ_LOCATION_1:
>> + return ®s->read_location1;
>> + case NAND_READ_LOCATION_2:
>> + return ®s->read_location2;
>> + case NAND_READ_LOCATION_3:
>> + return ®s->read_location3;
>> default:
>> return NULL;
>> }
>> @@ -590,6 +616,10 @@ static void update_rw_regs(struct qcom_nand_host
>> *host, int num_cw, bool read)
>> nandc_set_reg(nandc, NAND_FLASH_STATUS, host->clrflashstatus);
>> nandc_set_reg(nandc, NAND_READ_STATUS, host->clrreadstatus);
>> nandc_set_reg(nandc, NAND_EXEC_CMD, 1);
>> +
>> + if (read)
>> + nandc_set_readl(nandc, 0, 0, host->use_ecc ?
>> + host->cw_data : host->cw_size, 1);
>> }
>> /*
>> @@ -835,6 +865,10 @@ static void config_nand_page_read(struct
>> qcom_nand_controller *nandc)
>> */
>> static void config_nand_cw_read(struct qcom_nand_controller *nandc)
>> {
>> + if (nandc->props->is_bam)
>> + write_reg_dma(nandc, NAND_READ_LOCATION_0, 4,
>> + NAND_BAM_NEXT_SGL);
>> +
>> write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
>> write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
>> @@ -923,6 +957,7 @@ static int nandc_param(struct qcom_nand_host
>> *host)
>> nandc_set_reg(nandc, NAND_DEV_CMD1_RESTORE, nandc->cmd1);
>> nandc_set_reg(nandc, NAND_DEV_CMD_VLD_RESTORE, nandc->vld);
>> + nandc_set_readl(nandc, 0, 0, 512, 1);
>> write_reg_dma(nandc, NAND_DEV_CMD_VLD, 1, 0);
>> write_reg_dma(nandc, NAND_DEV_CMD1, 1, NAND_BAM_NEXT_SGL);
>> @@ -1398,6 +1433,19 @@ static int read_page_ecc(struct qcom_nand_host
>> *host, u8 *data_buf,
>> oob_size = host->ecc_bytes_hw + host->spare_bytes;
>> }
>> + if (nandc->props->is_bam) {
>> + if (data_buf && oob_buf) {
>> + nandc_set_readl(nandc, 0, 0, data_size, 0);
>> + nandc_set_readl(nandc, 1, data_size,
>> + oob_size, 1);
>> + } else if (data_buf) {
>> + nandc_set_readl(nandc, 0, 0, data_size, 1);
>> + } else {
>> + nandc_set_readl(nandc, 0, data_size,
>> + oob_size, 1);
>> + }
>> + }
>> +
>> config_nand_cw_read(nandc);
>> if (data_buf)
>> @@ -1457,6 +1505,7 @@ static int copy_last_cw(struct qcom_nand_host
>> *host, int page)
>> set_address(host, host->cw_size * (ecc->steps - 1), page);
>> update_rw_regs(host, 1, true);
>> + nandc_set_readl(nandc, 0, 0, size, 1);
>
> The READ_LOC reg should already be set up in update_rw_regs, right? If
> so, we could
> drop this line.
Yes. this line is redundant. I will remove the same.
>
> With that,
>
> Reviewed-by: Archit Taneja <architt@...eaurora.org>
>
> Thanks,
> Archit
>
>> config_nand_single_cw_page_read(nandc);
>> @@ -1502,6 +1551,7 @@ static int qcom_nandc_read_page_raw(struct
>> mtd_info *mtd,
>> u8 *data_buf, *oob_buf;
>> struct nand_ecc_ctrl *ecc = &chip->ecc;
>> int i, ret;
>> + int read_loc;
>> data_buf = buf;
>> oob_buf = chip->oob_poi;
>> @@ -1527,6 +1577,20 @@ static int qcom_nandc_read_page_raw(struct
>> mtd_info *mtd,
>> oob_size2 = host->ecc_bytes_hw + host->spare_bytes;
>> }
>> + if (nandc->props->is_bam) {
>> + read_loc = 0;
>> + nandc_set_readl(nandc, 0, read_loc, data_size1, 0);
>> + read_loc += data_size1;
>> +
>> + nandc_set_readl(nandc, 1, read_loc, oob_size1, 0);
>> + read_loc += oob_size1;
>> +
>> + nandc_set_readl(nandc, 2, read_loc, data_size2, 0);
>> + read_loc += data_size2;
>> +
>> + nandc_set_readl(nandc, 3, read_loc, oob_size2, 1);
>> + }
>> +
>> config_nand_cw_read(nandc);
>> read_data_dma(nandc, reg_off, data_buf, data_size1, 0);
>>
Powered by blists - more mailing lists