[RFC,v3 5/5] mtd: spinand: skip set/get oob data bytes when interleaved case
Miquel Raynal
miquel.raynal at bootlin.com
Mon Nov 22 01:01:44 PST 2021
Hi xiangsheng,
Thanks for the figures, they are useful too understand better your
situation.
xiangsheng.hou at mediatek.com wrote on Fri, 12 Nov 2021 16:33:34
+0800:
> Hi Miquel,
>
> Firstly, I would like to introduce Mediatek HW ECC engine how it
> organize the whole nand page data.
>
> Take page size 2KB and OOB size 64B for example,
>
> nand page standard data format:
> +---------------------------+------------+
> | | |
> | main area | oob area |
> | | |
> +---------------------------+------------+
>
> Mediatek HW ECC pipelined data format(sector size 1KB):
> +------------+-------+-----------+-------+
> | | | | |
> | sector(0) | oob(0)| sector(1)| oob(1)|
> | | | | |
> +------------+-------+-----------+-------+
>
> Mediatek HW ECC engine organize data as sector in unit.
> The sector size can be 512 or 1024 bytes.
> The OOB free and ecc bytes are stored right after the sector(n)
> main data.
>
> oob(n) layout:
> +-------+---------+-----------+
> | | | |
> | part1 | part2 | part3 |
> | | | |
> +-------+---------+-----------+
> part1: OOB bytes will be protected by ecc engine which called FDM ECC
Let's call this protected free OOB bytes
> part2: OOB bytes not be protected by ecc engine
Let's call this unprotected free OOB bytes
> part3: OOB bytes to store ecc parity for (sector data + FDM ECC bytes)
Let's call this ECC bytes
>
> part1 and part2 called FDM(flash disk manage data) which can be used to
> store BBM or filesystem manage data(like jffs2).
>
> The FDM and FDM ECC can be configurable,
> FDM number of bytes: 0 ~ 8 bytes
> FDM ECC number of byte: 0 ~ FDM size
>
> Therefore, the mtk ecc driver need to handle BBM swap and OOB shift
> operation before/after the write/read operation in
> ecc_finish/prepare_io_req.
Not necessarily. What if you just provide a translation in
prepare/finish which basically switches from one representation to the
other?
Whatever operation you try to do, we don't really care where all these
sections will be located once in memory, do we?
>
> 1. Why need BBM swap operation in mtk ecc driver?
>
> Ensure the BBM poisition consistent with nand specification.
>
> main area oob area
> +---------------------------+------------+
> | |
> |
> | |* |
> |
> | |
> +---------------------------+------------+
>
> sector(0) oob(0) sector(1) oob(1)
> +------------+-------+-----------+-------+
> | | | | |
> | | | # | |
> | | | | |
> +------------+-------+-----------+-------+
>
> (*): stand for the BBM
> (#): stand for one byte main data
>
> For the 2KB + 64B page case, the standard BBM position will be main
> data in Mediatek HW ECC data format. Therefore, the BBM swap between
> the BBM with one bye main data in sector(1).
>
> The data struct when BBM swap:
>
> sector(0) oob(0) sector(1) oob(1)
> +------------+-------+-----------+-------+
> | | |
> | |
> | | | * |# |
> | |
> | | |
> +------------+-------+-----------+-------+
>
> 2. Why need OOB shift opration in mtk ecc driver?
>
> Since the BBM located at oob(1) 1st byte in mtk ecc data format, the
> standard BBM position need at the 1st in the whole OOB area logically.
>
> Just take the data flow in prepare_io_req when write
> with MTD_OPS_AUTO_OOB for example:
>
> source: data buf oob buf
> +---------------------------+------------+
> | |
> |
> | | |
> |
> | |
> +---------------------------+------------+
>
> 1st: mtd_ooblayout_set_databytes
> data buf oob buf
> +---------------------------+------------+
> | |
> |
> | |*@@@@@@ |
> |
> | |
> +---------------------------+------------+
> (*): BBM, is reserve byte when set databyte
> (@): the free OOB data byte
>
> 2nd: BBM swap
>
> data buf oob buf
> sector(0) sector(1)
> +---------------------------+------------+
> | | |
> |
> | | * |#@@@@@@ |
> | |
> | |
> +---------------------------+------------+
> (#): stand for one byte main data
> (*): stand for the BBM
>
> 3rd: sector OOB shift
>
> data buf oob buf
> sector(0) sector(1) oob(1) oob(0)
> +---------------------------+------------+
> | | |
> | |
> | | * |@@@ |#@@ |
> | |
> | | |
> +---------------------------+------------+
>
> The mtk ECC engine will auto place the data struct on flash
> as bellow finally.
>
> sector(0) oob(1) sector(1) oob(0)
> +------------+-------+-----------+-------+
> | | |
> | |
> | |@@@ | * |#@@ |
> | |
> | | |
> +------------+-------+-----------+-------+
>
> The read data flow in finish_io_req is reverse with prepare_io_req when
> write.
>
> On Tue, 2021-11-09 at 13:05 +0100, Miquel Raynal wrote:
> > Hi Xiangsheng,
> >
> > Has we discussed a while ago:
> >
> > ...it is possible that I did not test with MTD_OPS_AUTO_OOB
> > recently and indeed the ECC bytes will missing in this case. In
> > the write path, maybe the ->prepare_io hooks should spread the
> > user data following req->mode in req.oobbuf before computing
> > the codes. Similar logic should be applied to the read path.
> >
> > Can you please add a patch for this situation in your next iteration?
> > It does not look like this situation has been handled.
> >
>
> As talked above, I may put the set/get OOB data bytes to each ecc
> driver not at the spinand driver.
>
> This may have some advantage:
> 1) Some ECC engine may protect the OOB bytes not only the main data.
> They will have same issue when read/write with MTD_OPS_AUTO_OOB.
> For this case, the OOB data bytes must set/get in
> prepare/finish_io_req.
>
> 2) For the syndrome layout, the data format on the flash may not be
> consistent with nand specification. It`s better handled by the special
> ecc engine.
>
> Can you agree with this modification?
> And, I will work on this and send in next iteration.
>
> > xiangsheng.hou at mediatek.com wrote on Fri, 22 Oct 2021 10:40:21 +0800:
> >
> > > For syndrome layout, ECC/free byte in oob layout and main
> > > data are interleaved. For this case, it is better to set/get
> > > oob data bytes in ECC engine.
> >
> > I don't understand the last sentence
>
> Just as the discussion talked above.
>
> > >
> > > For MTK ECC engine, although it can auto place data as sector +
> > > oob free + oob ecc for one page in pipelined. However, the bad
> > > mark will be not fit with nand spec. Therefore, there have bad
> > > mark swap operation in ecc engine.
> > >
> > > But, the swap opeation(between bbm 0xff with 1byte main data) will
> > > lead to more one byte than oobavailable.
> >
> > Sorry but again, I don't understand what you mean.
> >
>
> data buf oob buf
> +---------------------------+------------+
> | | |
> |
> | | * |#@@@@@@ |
> | |
> | |
> +---------------------------+------------+
> (#): stand for one byte main data
> (*): stand for the BBM
> (@): the free OOB data byte
>
> Take write ooblen is mtd->oobavail for example,
> the data in standard OOB area will be one more main data byte, due to
> the BBM swap operation, lead to the expected OOB len as
> (mtd->oobavail + 1). This is not as expected for one page.
>
> > > Set oob databytes after
> > > bad mark swap will lead to lost one oob free byte.
> >
> > We don't care about free OOB bytes really.
> >
>
> The MTD_OPS_AUTO_OOB need handle the free OOB bytes. Some filesystem
> (like jffs2) also need store data at free OOB for management.
>
> > >
> > > Therefore, just try to modify the spi nand framework for review.
> > > And this may be common for the interleaved case.
> >
> > I don't get how falling back to a memcpy will solve your problem? Can
> > you please provide an anscii figure or something more visual to let
> > us
> > understand?
>
> As talked above, the BBM swap and OOB shift handled at mtk ecc driver.
> And the mtk controller will auto place data as mtk ECC data format in
> pipelined.
>
> Thanks
> Xiangsheng Hou
>
>
>
>
>
Thanks,
Miquèl
More information about the Linux-mediatek
mailing list