* How read_oob() should work for HW_SYNDROME NAND controller?
@ 2016-11-30 7:05 Masahiro Yamada
2016-11-30 7:15 ` Boris Brezillon
2016-12-01 9:09 ` Masahiro Yamada
0 siblings, 2 replies; 4+ messages in thread
From: Masahiro Yamada @ 2016-11-30 7:05 UTC (permalink / raw)
To: Boris Brezillon, linux-mtd
Cc: Marek Vasut, Brian Norris, Andy Shevchenko, Jason Roberts,
David Woodhouse, Chuanxiao Dong, Dinh Nguyen, Artem Bityutskiy
Hi.
(CCing Intel engineers because this is related to Denali driver
and comments from them are very appreciated.)
I am tacking on Denali NAND driver rework.
My question is:
Which data should chip->ecc.read_oob() get
in case of a controller with syndrome-like ECC engine.
For example, say we have a NAND chip
with 2048 byte page + 64 byte oob.
(the picture in the right side.)
And let's say the controller's ecc.size = 1024 and ecc.bytes == 14.
I am omitting BBM to make the situation simpler.
The Denali NAND controller interleaves
payload and ECC like the picture in the left side.
|-----------| |-----------|
| | | |
| Payload0 | | |
| | | |
| (ecc.size | | |
| 1024B) | | Main Page |
| | | area |
|-----------| | |
| ECC0 | | 2048B |
| (ecc.bytes| | |
| 14B) | | |
|-----------| | |
| | | |
| Payload1 | | |
| | | |
| (ecc.size | | |
| 1024B) | |-----------|
| | | |
|-----------| | |
| ECC1 | | OOB area |
| (ecc.bytes| | |
| 14B) | | 64B |
|-----------| | |
| OOB free | | |
| 36B | | |
|-----------| |-----------|
The controller can transfer
Main page area, OOB area, or both of them,
like the physical structure of the device
in the right-side picture.
This is different from how
Denali controller uses the page logically
(in the left-side picture).
The current read_oob() implementation in the Denali driver
simply get the data in the physical OOB area.
It corresponds the tail of Payload1 + ECC1 + OOB free.
I am afraid the behavior is different from
the reference implementation of
nand_read_page_raw_syndrome()
nand_read_oob_syndrome()
I think we should collect ECC sections
into oob_poi to get along with the ooblayout APIs.
The Denali IP also supports lowlevel
command-base interface to issue NAND_CMD_RNDOUT
and cherry-pick ECC sections.
But, more simply, I can transfer the whole page + oob
into a temporary buffer, then only copy
ECC sections into oob_poi.
Comments are very appreciated.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: How read_oob() should work for HW_SYNDROME NAND controller?
2016-11-30 7:05 How read_oob() should work for HW_SYNDROME NAND controller? Masahiro Yamada
@ 2016-11-30 7:15 ` Boris Brezillon
2016-12-01 9:09 ` Masahiro Yamada
1 sibling, 0 replies; 4+ messages in thread
From: Boris Brezillon @ 2016-11-30 7:15 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-mtd, Marek Vasut, Brian Norris, Andy Shevchenko,
Jason Roberts, David Woodhouse, Chuanxiao Dong, Dinh Nguyen,
Artem Bityutskiy
On Wed, 30 Nov 2016 16:05:36 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Hi.
> (CCing Intel engineers because this is related to Denali driver
> and comments from them are very appreciated.)
>
> I am tacking on Denali NAND driver rework.
>
> My question is:
> Which data should chip->ecc.read_oob() get
> in case of a controller with syndrome-like ECC engine.
>
> For example, say we have a NAND chip
> with 2048 byte page + 64 byte oob.
> (the picture in the right side.)
>
> And let's say the controller's ecc.size = 1024 and ecc.bytes == 14.
> I am omitting BBM to make the situation simpler.
Hm, actually the placement of the BBM is important. Do you know where
it's placed in the page layout used by Denali.
>
> The Denali NAND controller interleaves
> payload and ECC like the picture in the left side.
>
> |-----------| |-----------|
> | | | |
> | Payload0 | | |
> | | | |
> | (ecc.size | | |
> | 1024B) | | Main Page |
> | | | area |
> |-----------| | |
> | ECC0 | | 2048B |
> | (ecc.bytes| | |
> | 14B) | | |
> |-----------| | |
> | | | |
> | Payload1 | | |
> | | | |
> | (ecc.size | | |
> | 1024B) | |-----------|
> | | | |
> |-----------| | |
> | ECC1 | | OOB area |
> | (ecc.bytes| | |
> | 14B) | | 64B |
> |-----------| | |
> | OOB free | | |
> | 36B | | |
> |-----------| |-----------|
>
> The controller can transfer
> Main page area, OOB area, or both of them,
> like the physical structure of the device
> in the right-side picture.
>
> This is different from how
> Denali controller uses the page logically
> (in the left-side picture).
>
>
> The current read_oob() implementation in the Denali driver
> simply get the data in the physical OOB area.
> It corresponds the tail of Payload1 + ECC1 + OOB free.
>
>
> I am afraid the behavior is different from
> the reference implementation of
> nand_read_page_raw_syndrome()
> nand_read_oob_syndrome()
Yep, you got it right, the read_oob() method should abstract away the
internal/controller-dependent page layout. In this specific case, you
should read each ECC section and the OOB free section.
>
>
> I think we should collect ECC sections
> into oob_poi to get along with the ooblayout APIs.
>
>
> The Denali IP also supports lowlevel
> command-base interface to issue NAND_CMD_RNDOUT
> and cherry-pick ECC sections.
>
> But, more simply, I can transfer the whole page + oob
> into a temporary buffer, then only copy
> ECC sections into oob_poi.
It should be faster if you only retrieve ECC sections (less I/Os), but
that's just optimization. Note that read_oob() is heavily used when
scanning bad blocks, so it might make a huge boot-time difference in the
end.
Regards,
Boris
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: How read_oob() should work for HW_SYNDROME NAND controller?
2016-11-30 7:05 How read_oob() should work for HW_SYNDROME NAND controller? Masahiro Yamada
2016-11-30 7:15 ` Boris Brezillon
@ 2016-12-01 9:09 ` Masahiro Yamada
2016-12-01 9:15 ` Boris Brezillon
1 sibling, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2016-12-01 9:09 UTC (permalink / raw)
To: Boris Brezillon, linux-mtd
Cc: Marek Vasut, Brian Norris, Andy Shevchenko, Jason Roberts,
David Woodhouse, Chuanxiao Dong, Dinh Nguyen, Artem Bityutskiy,
masahiroy
Hi Boris,
>>
>> And let's say the controller's ecc.size = 1024 and ecc.bytes == 14.
>> I am omitting BBM to make the situation simpler.
>
> Hm, actually the placement of the BBM is important. Do you know where
> it's placed in the page layout used by Denali.
BBM is placed at the beginning of the OOB area.
(so, factory BBMs are protected from the ECC correction engine.)
The more precise layout is as follows:
|-----------| |-----------|
| | | |
| Payload0 | | |
| | | |
| (ecc.size | | |
| 1024B) | | Main Page |
| | | area |
|-----------| | |
| ECC0 | | 2048B |
| (ecc.bytes| | |
| 14B) | | |
|-----------| | |
| | | |
| Payload1 | | |
| | | |
| (ecc.size | | |
| 1010B) | | |
|-----------| |-----------|
| BBM (8B) | | |
|-----------| | |
| Payload1 | | |
| 14B | | |
|-----------| | |
| ECC1 | | OOB area |
| (ecc.bytes| | |
| 14B) | | 64B |
|-----------| | |
| OOB free | | |
| 28B | | |
|-----------| |-----------|
The Playload1 is split by the BBM area.
>> The Denali IP also supports lowlevel
>> command-base interface to issue NAND_CMD_RNDOUT
>> and cherry-pick ECC sections.
>>
>> But, more simply, I can transfer the whole page + oob
>> into a temporary buffer, then only copy
>> ECC sections into oob_poi.
>
> It should be faster if you only retrieve ECC sections (less I/Os), but
> that's just optimization. Note that read_oob() is heavily used when
> scanning bad blocks, so it might make a huge boot-time difference in the
> end.
I think this comes down to
"PIO register access only for ECC sectors"
vs
"DMA transfer for the whole page"
I will test which is better.
BTW, surprisingly enough, the Denali still sets
NAND_SKIP_BBTSCAN flag.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: How read_oob() should work for HW_SYNDROME NAND controller?
2016-12-01 9:09 ` Masahiro Yamada
@ 2016-12-01 9:15 ` Boris Brezillon
0 siblings, 0 replies; 4+ messages in thread
From: Boris Brezillon @ 2016-12-01 9:15 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-mtd, Marek Vasut, Brian Norris, Andy Shevchenko,
Jason Roberts, David Woodhouse, Chuanxiao Dong, Dinh Nguyen,
Artem Bityutskiy, masahiroy
On Thu, 1 Dec 2016 18:09:37 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Hi Boris,
>
>
> >>
> >> And let's say the controller's ecc.size = 1024 and ecc.bytes == 14.
> >> I am omitting BBM to make the situation simpler.
> >
> > Hm, actually the placement of the BBM is important. Do you know where
> > it's placed in the page layout used by Denali.
>
>
> BBM is placed at the beginning of the OOB area.
> (so, factory BBMs are protected from the ECC correction engine.)
>
>
> The more precise layout is as follows:
>
> |-----------| |-----------|
> | | | |
> | Payload0 | | |
> | | | |
> | (ecc.size | | |
> | 1024B) | | Main Page |
> | | | area |
> |-----------| | |
> | ECC0 | | 2048B |
> | (ecc.bytes| | |
> | 14B) | | |
> |-----------| | |
> | | | |
> | Payload1 | | |
> | | | |
> | (ecc.size | | |
> | 1010B) | | |
> |-----------| |-----------|
> | BBM (8B) | | |
> |-----------| | |
> | Payload1 | | |
> | 14B | | |
> |-----------| | |
> | ECC1 | | OOB area |
> | (ecc.bytes| | |
> | 14B) | | 64B |
> |-----------| | |
> | OOB free | | |
> | 28B | | |
> |-----------| |-----------|
>
>
> The Playload1 is split by the BBM area.
Okay, thanks for the detailed layout.
Everything looks good, and you should be able to disable the BBT and
force BBM scanning.
>
>
>
>
> >> The Denali IP also supports lowlevel
> >> command-base interface to issue NAND_CMD_RNDOUT
> >> and cherry-pick ECC sections.
> >>
> >> But, more simply, I can transfer the whole page + oob
> >> into a temporary buffer, then only copy
> >> ECC sections into oob_poi.
> >
> > It should be faster if you only retrieve ECC sections (less I/Os), but
> > that's just optimization. Note that read_oob() is heavily used when
> > scanning bad blocks, so it might make a huge boot-time difference in the
> > end.
>
>
> I think this comes down to
>
> "PIO register access only for ECC sectors"
> vs
> "DMA transfer for the whole page"
>
> I will test which is better.
>
>
> BTW, surprisingly enough, the Denali still sets
> NAND_SKIP_BBTSCAN flag.
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-01 9:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-30 7:05 How read_oob() should work for HW_SYNDROME NAND controller? Masahiro Yamada
2016-11-30 7:15 ` Boris Brezillon
2016-12-01 9:09 ` Masahiro Yamada
2016-12-01 9:15 ` Boris Brezillon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).