lkml.org 
[lkml]   [2018]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 13/16] mtd: rawnand: qcom: minor code reorganization for bad block check
On 2018-05-28 12:33, Miquel Raynal wrote:
> Hi Abhishek,
>
>> >> /* 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?
>>
>> All the layouts are such that, the BBM will come in
>> first byte of spare area.
>>
>> For 4 bit ECC, the cw_size is 528 so for 2K page
>>
>> 2048 - 528 * 3 = 464
>
> My question was more about small page NANDs. But I suppose it works
> too if ecc->steps == 1.
>

Correct Miquel.

>>
>> So for last CW, the 464 is BBM (i.e 2048th byte) in
>> full page.
>>
>> > >> >> 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]?
>> >
>> The main thing here is
>> nandc_set_read_loc(nandc, 0, bbpos, host->bbm_size, 1);
>>
>> After reading one complete CW from NAND, the data will be still
>> in NAND HW buffer.
>>
>> The above register tells that we need to read data from
>> bbpos of size host->bbm_size (which is 1 byte for 8 bus witdh
>> and 2 byte for 16 bus width) in bbm_bytes_buf.
>
> I see: idx 0 in bbm_bytes_buf is the data at offset bbpos. Then
> it's ok.
>
>>
>> So bbm_bytes_buf[0] will contain the BBM first byte.
>> and bbm_bytes_buf[1] will contain the BBM second byte.
>>
>> Regards,
>> Abhishek
>>
>> >> >> if (chip->options & NAND_BUSWIDTH_16)
>> >> - bad = bad || (nandc->data_buffer[bbpos + 1] != 0xff);
>> >> + bad = bad || (bbm_bytes_buf[1] != 0xff);
>
> Sorry, my mistake, I did not see the above line.
>
> However, technically, the BBM could be located in the first, second or
> last page of the block. You should check the three of them are 0xFF
> before declaring the block is not bad.
>
> The more I look at the function, the more I wonder if you actually need
> it. Why does the generic nand_block_bad() implementation in the core
> do not fit?

The BBM bytes can be accessed in raw mode only for QCOM NAND
Contoller. We started with following patch for initial patches

http://patchwork.ozlabs.org/patch/508565/

I am also not very much sure, how can we go ahead now.
Ideally we need to use generic function only which
requires raw_read.

Thanks,
Abhishek

\
 
 \ /
  Last update: 2018-05-28 18:24    [W:0.107 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site