On Fri, 20 Jul 2018 17:46:38 +0530
Abhishek Sahu [off-list ref] wrote:

Hi Boris,

On 2018-07-19 03:13, Boris Brezillon wrote:

quoted

On Wed, 18 Jul 2018 23:23:50 +0200
Miquel Raynal [off-list ref] wrote:
  

quoted

Boris,

Can you please check the change in qcom_nandc_write_oob() is
valid? I think it is but as this is a bit of a hack I prefer double 
checking.  

Indeed, it's hack-ish.
  

quoted

Thanks,
Miquèl


Abhishek Sahu [off-list ref] wrote on Fri,  6 Jul 2018
13:21:56 +0530:
  

quoted

The NAND base layer calls write_oob() by setting bytes at
chip->badblockpos with value non 0xFF for updating bad block status.
The QCOM NAND controller skips the bad block bytes while doing normal
write with ECC enabled. When initial support for this driver was
added, the driver specific function was added temporarily for
block_markbad() with assumption to change for raw read in NAND base
layer. Moving to raw read for block_markbad() seems to take more time
so this patch removes driver specific block_markbad() function by
using following HACK in write_oob() function.

Check for BBM bytes in OOB and accordingly do raw write for updating
BBM bytes in NAND flash or normal write for updating available OOB
bytes.  

Why don't we change that instead of patching the qcom driver to guess
when the core tries to mark a block bad? If you're afraid of breaking
existing drivers that might rely on the "write/read BBM in non-raw
mode" solution (I'm sure some drivers are), you can always add a new
flag in chip->options (NAND_ACCESS_BBM_IN_RAW_MODE) and only use raw
accessors when this flag is set.
  

  We started with that Only

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

  and since we didn't conclude, we went for driver
  specific bad block check and mark bad block functions.

  Now, we wanted to get rid of driver specific functions

  1. For bad block check, we found the way to get the BBM bytes
     with ECC read. Controller updates BBM in separate register
     which we can read and update the same in OOB. Patch #1 of
     series does the same.

  2. For bad block mark, there is no way to update in ECC mode
     that's why we went for HACK to get rid of driver specific
     function.

  If adding flag is fine now then this HACK won't be required.

Yep. I'm fine with that. Can you rebase the patch you pointed out on top
of nand/next and move the flag to chip->options instead of
chip->bbt_options + prefix it with NAND_ instead of NAND_BBT_?

  Moving to RAW mode for every one still looks risky.

I agree.