Re: [PATCH 6/9] mtd: nand: qcom: support for checking read errors for last codeword

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Abhishek,

On Mon, 23 Apr 2018 11:38:27 +0530, Abhishek Sahu
<absahu@codeaurora.org> wrote:

> On 2018-04-22 21:45, Miquel Raynal wrote:
> > Hi Abhishek,  
> > > On Thu, 12 Apr 2018 12:47:42 +0530, Abhishek Sahu  
> > <absahu@codeaurora.org> wrote:  
> > >> On 2018-04-10 15:35, Miquel Raynal wrote:
> >> > Hi Abhishek,  
> >> > > On Wed,  4 Apr 2018 18:12:22 +0530, Abhishek Sahu  
> >> > <absahu@codeaurora.org> wrote:  
> >> > >> Add boolean function argument in parse_read_errors to identify  
> >> >> whether the read error has been called for complete page read or
> >> >> only last codeword read. This will help in subsequent patches to
> >> >> detect ECC errors in case of last codeword read.
> >> > > Can you explain when this happen: "last codeword read"? I don't see the  
> >> > use case.  
> >> >>   Hi Miquel,
> >> >>   This is happening inside qcom_nandc_write_oob where the last subpage  
> >>   data is being copied first.
> > > I still don't understand the use case.
> > > What to you mean last 'subpage copied first'?  
> >   
>   Hi Miquel,
> 
>   According to current implementation
> 
>   QCOM NAND layout protect 16 bytes of available oob with ECC also.
>   When ecc->write_oob (qcom_nandc_write_oob) is being called
>   then it can't update just OOB bytes.
> 
>   It needs to first read the last subpage which includes old
>   OOB bytes. Then it updates the old OOB bytes with new one
>   and then again write the data back.
> 
>   You can refer function comment of qcom_nandc_write_oob
>   for the same.

Oh well, I think I see where we misunderstood: ->write_oob only asks
you to write some data, that is all. The user is supposed to know
which type of flash he is handling. If he writes a section that
has already been written, well, bad for him. But you certainly don't
want to handle that in the controller driver directly.

> 
>   But, to me, it looks like this read is unnecessary since
>   all the other bytes will be 0xff only. Require your help
>   in confirming the same and then I will remove that read
>   last subpage implementation.

I think you can drop it.

Thanks,
Miquèl

> 
>   Thanks,
>   Abhishek
> 
> >> >>     host->use_ecc = true;
> >> >>     clear_bam_transaction(nandc);  
> >>     ret = copy_last_cw(host, page);
> >>     if (ret)
> >>         return ret;  
> >> >>   you can refer function comment of qcom_nandc_write_oob  for more  
> >>   detail.  
> >> >>   Thanks,  
> >>   Abhishek  



-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com