Re: [PATCH v3 4/4] ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

2012-12-06 Thread Ivan Djelic 
On Wed, Dec 05, 2012 at 05:15:52AM +, Philip, Avinash wrote:
(...)
  First a short reminder of pros and cons of the constant polynomial 
  addition
  (let's just call it CPA) feature:
  
  pros:
  - it elegantly solves all problems related to reading an erased page (no 
  clumsy hack needed)
  - better performance: when a bitflip appears on an erased page (often this 
  is a sticky bitflip),
there is no need to perform a full scan+cleanup of the page each time it 
  is read
 
 No need for scan + cleanup on each read, as the chances of encountering bit 
 flips in erased page
 is less. Also to find bit flips in erased page, compare ecc vector for read 
 erased page against
 a standard ecc vector. Presence of bit flips can find by checking the compare 
 results. In case of
 mismatch, should go for correction of bit flips in erased page with full scan.

Hi Avinash,

I explicitly mentioned the condition when a bitflip appears on an erased 
page, in which case you
_do_ have to do a full scan upon each read, until you erase the block; and 
then, the bitflip may still be there
(this is what I called a sticky bitflip).
My experience with recent devices (4-bit/8-bit) is that erased pages with a 
single bitflip are not uncommon.
For those pages, there is undeniably a performance penalty compared to CPA.
Benchmarks would be needed to quantify the overall performance impact, but I 
suspect it is small.

 
 So with this approach, we can nullify the effect of CPA for erased page bit 
 flip handling.

Well, not completely. I would happily drop CPA is that were the case.
 
  
  cons:
  - the ecc vector is not compatible with RBL
  
  RBL compatibility is not necessary in my case, because I'm using the OMAP 
  MLC ROM boot mode.
  Rather than completely removing the CPA feature, I'd like to keep it as an 
  option; it could
  even be used with the ELM module.
 
 I agree RBL compatibility depends on the user. But with RBL compatibility, 
 there is no sacrifice
 of any existing feature. Is it right?
 
 Also nand driver get simplified with removal of CPA, so that both HW  SW 
 error correction
 can go for same ecc calculation.

Indeed that would be a simplification.

  
  I'm OK to submit a patch in this direction, but first I'd like to wait for 
  the dust to settle
  on arch/arm/mach-omap2 and mtd/nand/omap2.c with Afzal patches and 
  everything; things have become
  a bit complicated to follow recently :-)
 
 Afzal's changes are in  are settled now.

What about this set: 
http://lists.infradead.org/pipermail/linux-mtd/2012-October/044337.html
I cannot see it in l2-mtd-2.6 ? Or did I miss something ?

Thanks,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/4] ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

2012-12-01 Thread Ivan Djelic 
On Wed, Nov 28, 2012 at 05:01:30AM +, Philip, Avinash wrote:
(...)
 
 Daniel,
 
 So can you start supporting bch8-am335xrbl-compatible and remove usage of
 is_elm_used in dt. This should come in ecc_opt field.
 
 In omap2 NAND driver, AM335x RBL compatibility is achieved depending on 
 ecc_layout
 and runtime detection of elm module. So options related to can be
 1. bch8-am335xrbl-compatible is enabled and run time detection of
Of elm module passed, RBL compatibility achieved.
 2. bch8-am335xrbl-compatible is enabled and run time detection of
of elm module failed, RBL compatibility sacrificed but continue with
software BCH8 error correction. Sacrificing RBL compatibility
because of constant polynomial addition and usage of 14 byte instead of 13 
 byte.
 
 Ivan,
 Do you have any plan of removing addition of constant polynomial to ecc data
 and go for extra byte checking to find erased pages?
 This way we can start support BCH8 with RBL compatible and kernel
 Didn't put any restriction of the ecc layout.

Hello Avinash,

Sorry about the response delay.
First a short reminder of pros and cons of the constant polynomial addition
(let's just call it CPA) feature:

pros:
- it elegantly solves all problems related to reading an erased page (no clumsy 
hack needed)
- better performance: when a bitflip appears on an erased page (often this is a 
sticky bitflip),
  there is no need to perform a full scan+cleanup of the page each time it is 
read

cons:
- the ecc vector is not compatible with RBL

RBL compatibility is not necessary in my case, because I'm using the OMAP MLC 
ROM boot mode.
Rather than completely removing the CPA feature, I'd like to keep it as an 
option; it could
even be used with the ELM module.

I'm OK to submit a patch in this direction, but first I'd like to wait for the 
dust to settle
on arch/arm/mach-omap2 and mtd/nand/omap2.c with Afzal patches and everything; 
things have become
a bit complicated to follow recently :-)
Also, I think it would be nice to move BCH code out of omap2.c into a separate 
file.
What do you think ?

BR,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] mtd: omap: nand: Remove 0xFF's that prefixed 16bit NAND addresses

2012-11-15 Thread Ivan Djelic 
On Thu, Nov 15, 2012 at 03:18:44PM +, Artem Bityutskiy wrote:
 On Thu, 2012-11-15 at 09:48 -0500, Christopher Harvey wrote:
  On Thu, Nov 15, 2012 at 01:02:09PM +0200, Artem Bityutskiy wrote:
   On Mon, 2012-10-29 at 15:51 -0400, Christopher Harvey wrote:
In 16bit NAND mode the GPMC would send the address 0xNN as 0xFFNN
instead of 0x00NN on the bus. The 0xFFs were actually uninitialized
bits that were left unset in the GPMC command output register. The
reason they weren't initialized in 16bit mode is that if the same code
that writes to this register was used in 8bit mode then 2 commands
would be output in 8bit mode. One for the low byte, and an extra 0x0
command for the high byte. This commit uses writew if we're using
16bit NAND. This commit also changes the high byte in the command
output register, but they are ignored by NAND chips anyway.

Most chips seem fine with the extra 0xFFs, but the ONFI spec says
otherwise.

Signed-off-by: Christopher Harvey char...@matrox.com
   
   Pushed to l2-mtd.git, thanks!
  
  !!! Did anybody get around to testing this? I thought this patch had
  been abandoned. Will testing get done on an omap chip now that it
  is in a tree?
  
  I should have prefixed it with RFC.
 
 I assume _you_ tested it, and Ivan was happy. But if it is untested, I
 am dropping it.

Unfortunately I can't test it at the moment,
BR,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] mtd: omap: nand: Remove 0xFF's that prefixed 16bit NAND commands

2012-10-29 Thread Ivan Djelic 
On Fri, Oct 26, 2012 at 08:36:43PM +0100, Christopher Harvey wrote:
 In 16bit NAND mode the GPMC would send the command 0xNN as 0xFFNN
 instead of 0x00NN on the bus. The 0xFFs were actually uninitialized
 bits that were left unset in the GPMC command output register. The
 reason they weren't initialized in 16bit mode is that if the same code
 that writes to this register was used in 8bit mode then 2 commands
 would be output in 8bit mode. One for the low byte, and an extra 0x0
 command for the high byte. This commit uses writew if we're using
 16bit NAND.
 
 Most chips seem fine with the extra 0xFFs, but the ONFI spec says
 otherwise.

Hi Christopher,

Nitpick: I think you should replace 'command' with 'address' in your commit 
message.
The ONFI spec says Host should send _address_ byte NN as 0x00NN. It is OK to 
send
command NN as 0xFFNN, as explicitly mentioned in ONFI 3.1 spec (section 2.16):

  2.16.  Bus Width Requirements
All NAND Targets per device shall use the same data bus width. All targets 
shall either have an
8-bit bus width or a 16-bit bus width. Note that devices that support the 
NV-DDR or NV-DDR2
data interface shall have an 8-bit bus width.
When the host supports a 16-bit bus width, only data is transferred at the 
16-bit width. All
address and command line transfers shall use only the lower 8-bits of the data 
bus. During
command transfers, the host may place any value on the upper 8-bits of the data 
bus. During
address transfers, the host shall set the upper 8-bits of the data bus to 00h.

Your patch deals with both command and address bytes, which does not hurt.
BR,
--
Ivan

 
 Signed-off-by: Christopher Harvey char...@matrox.com
 ---
  drivers/mtd/nand/omap2.c |   14 +-
  1 files changed, 9 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
 index 5b31386..ae6738f 100644
 --- a/drivers/mtd/nand/omap2.c
 +++ b/drivers/mtd/nand/omap2.c
 @@ -225,16 +225,20 @@ static void omap_hwcontrol(struct mtd_info *mtd, int 
 cmd, unsigned int ctrl)
  {
   struct omap_nand_info *info = container_of(mtd,
   struct omap_nand_info, mtd);
 + void __iomem *reg;
  
   if (cmd != NAND_CMD_NONE) {
   if (ctrl  NAND_CLE)
 - writeb(cmd, info-reg.gpmc_nand_command);
 -
 + reg = info-reg.gpmc_nand_command;
   else if (ctrl  NAND_ALE)
 - writeb(cmd, info-reg.gpmc_nand_address);
 -
 + reg = info-reg.gpmc_nand_address;
   else /* NAND_NCE */
 - writeb(cmd, info-reg.gpmc_nand_data);
 + reg = info-reg.gpmc_nand_data;
 +
 + if (info-nand.options  NAND_BUSWIDTH_16)
 + writew(cmd, reg);
 + else
 + writeb(cmd, reg);
   }
  }
  
 -- 
 1.7.8.6
 
 __
 Linux MTD discussion mailing list
 http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] mtd: nand: omap2: Add data correction support

2012-10-11 Thread Ivan Djelic 
On Thu, Oct 11, 2012 at 06:27:13AM +0100, Philip, Avinash wrote:
(...)
  Another simple strategy could use the fact that you add a 14th zero byte to
  the 13 BCH bytes for RBL compatibility:
 
 RBL compatibility (14th byte) is applicable only for BCH8 ecc scheme.
 
 So I am planning adding an extra byte (0) for BCH4 ecc scheme. So with this
 we can go for same approaches in BCH4  BCH8 ecc scheme.
 
 If I understood correctly, software BCH ecc scheme is modifying calculated
 ecc data to handle bit flips in erased pages.
 
 If that is the only reason, whether same logic can go for same ECC calculation
 (remove modification of calculated ecc in case of software ecc correction)
 by adding an extra byte (0) in spare area to handle erased pages.
 
 So can you share if I am missing something?

Yes, the only reason why a constant polynomial is added to hw-generated ECC 
bytes is to transparently handle bitflips in erased pages.
Handling erased pages this way has several benefits over the zero byte hack:
- cleaner code, no checking of the zero byte
- no expensive scan of data+spare area when reading an erased block: this step 
can significantly slow down the initial UBI scan (lots of erased pages to read)
- no need to worry about the (very unlikely) possibility of having more than 4 
bitflips in the zero byte

OTOH, having the same ECC codes for both ELM and non-ELM chips with RBL 
compatibility sounds nice and would also simplify things.
Note: on platforms where we use SW BCH correction, we also use the MLC OMAP 
boot mode, which is more robust and not compatible with 8-bit/4-bit BCH layouts.

I don't know which way is better for the OMAP community:
1. Unifying ECC modes = loosing the constant polynomial benefits, but gaining 
RBL compat and simplifying code
2. Keeping separate ECC modes = code bloat

Tony, do you have an opinion on this ?

BTW, Afzal is submitting a series of patches [1] which are not compatible with 
your series; is there any plan to merge your patches ?

BR,
--
Ivan

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-October/044374.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/14] OMAP-GPMC related cleanup for common zImage

2012-10-10 Thread Ivan Djelic 
On Mon, Oct 08, 2012 at 07:08:08AM +0100, Mohammed, Afzal wrote:
 Hi Ivan,
 
 On Mon, Oct 08, 2012 at 11:05:56, Mohammed, Afzal wrote:
 
  This series cleans up omap-gpmc related code so that omap can
  be a part of common zImage.
 
  This series moves gpmc.h from plat-omap/include/plat to mach-omap2
  so that header file is local.
 
  Patches 7  8 cleans up the already moved platform data header files
  to contain only platform data. Also gpmc-nand information is moved
  to nand platform data header.
  
  Patches 9-13 makes nand driver independent of gpmc header file
  
  And the final patch localizes gpmc header.
 
 BCH[48] support that you have added on OMAP using gpmc exported
 symbols has been changed such that nand driver now takes care
 of BCH support without relying on gpmc exported symbols.
 
 This is more or less a cut  paste of your implementation, which was
 necessitated now due to common ARM zImage cleanup w.r.t header files.
 
 Please verify that BCH[48] works as earlier with this series.

Hi Afzal,

I ran several mtd regression tests on a Beagle Board on your gpmc-czimage-v2 
tag.
All BCH error correcting tests passed successfully.

I occasionally had weird read errors though, especially when reading blank 
pages:
the omap driver returned 512-byte sectors containing something like:

30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff
30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff
30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff
30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff
30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff
30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff30ff











instead of:


















I was able to reproduce the problem also on l2-mtd tip, albeit less often.
The problem seems to occur quite randomly, it may be a hardware issue on
my board...

Anyway, the ECC handling part looks OK to me.

Best regards,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] mtd: nand: omap2: Add data correction support

2012-10-10 Thread Ivan Djelic 
On Tue, Oct 09, 2012 at 01:36:50PM +0100, Philip, Avinash wrote:
(...)
  There are at least 2 potential problems when reading an erased page with 
  bitflips:
  
  1. bitflip in data area and no bitflip in spare area (all 0xff)
  Your code will not perform any ECC correction.
  UBIFS does not like finding bitflips in empty pages, see for instance
  http://lists.infradead.org/pipermail/linux-mtd/2012-March/040328.html.
 
 In case of error correction using ELM, syndrome vector calculated after 
 reading
 Data area  OOB area. So handling of erased page requires a software 
 workaround.
 I am planning something as follows.
 
 I will first check calculated ecc, which would be zero for non error pages.
 Then I would check 0xFF in OOB area (for erased page) by checking number of
 bit zeros in OOB area.  If it is 0xFF (number of bit zero count is zero),
 set entire page as 0xFF if number of bit zeros is less than max bit flips
 (8 or 4) by counting the number of bit zero's in data area.
 
 This logic is implemented in fsmc_nand.c
 
 See commit
 mtd: fsmc: Newly erased page read algorithm implemented
 
  
  2. bitflip in ECC bytes in spare area
  Your code will report an uncorrectable error upon reading; if this happens 
  while reading a partially programmed UBI block,
  I guess you will lose data.
 
 In case of uncorrectable errors due to bit flips in spare area,
 I can go on checking number of bit zero's in data area + OOB area
 are less than max bit flips (8 or 4), I can go on setting the entire
 page as 0xFF.
 

OK, sounds reasonable.
Another simple strategy could use the fact that you add a 14th zero byte to
the 13 BCH bytes for RBL compatibility:

Upon reading:
 - if this 14th byte is zero (*) = page was programmed: perform ECC
   correction as usual
 - else, page was not programmed: do not perform ECC, read entire data+spare
   area, and set it to 0xff if less than 8 or 4 (max bitflips) zero bits
   were found

(*) for robustness to bitflip in 14th byte, replace condition
14th byte is zero by e.g. 14th byte has less than 4 bits set to 1.

What do you think ?

BR,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] mtd: nand: omap2: Add data correction support

2012-10-05 Thread Ivan Djelic 
On Fri, Oct 05, 2012 at 09:51:50AM +0100, Philip, Avinash wrote:
 On Thu, Oct 04, 2012 at 15:51:03, Philip, Avinash wrote:
  On Thu, Oct 04, 2012 at 00:50:45, Ivan Djelic wrote:
   On Wed, Oct 03, 2012 at 03:29:49PM +0100, Philip, Avinash wrote:
ELM module can be used for error correction of BCH 4  8 bit. Also
support read  write page in one shot by adding custom read_page 
write_page methods. This helps in optimizing code.

New structure member is_elm_used is added to know the status of
whether the ELM module is used for error correction or not.

Note:
ECC layout of BCH8 uses 14 bytes for 512 byte of data to make compatible
with RBL ECC layout, even though the requirement was only 13 byte. This
results a common ecc layout across RBL, U-boot  Linux.

   
   See a few comments below,
   
   (...)
+static int omap_elm_correct_data(struct mtd_info *mtd, u_char *dat,
+   u_char *read_ecc, u_char *calc_ecc)
+{
+   struct omap_nand_info *info = container_of(mtd, struct 
omap_nand_info,
+   mtd);
+   int eccsteps = info-nand.ecc.steps;
+   int i , j, stat = 0;
+   int eccsize, eccflag, size;
+   struct elm_errorvec err_vec[ERROR_VECTOR_MAX];
+   u_char *ecc_vec = calc_ecc;
+   enum bch_ecc type;
+   bool is_error_reported = false;
+
+   /* initialize elm error vector to zero */
+   memset(err_vec, 0, sizeof(err_vec));
+   if (info-nand.ecc.strength == BCH8_MAX_ERROR) {
+   size = BCH8_SIZE;
+   eccsize = BCH8_ECC_OOB_BYTES;
+   type = BCH8_ECC;
+   } else {
+   size = BCH4_SIZE;
+   eccsize = BCH4_SIZE;
+   type = BCH4_ECC;
+   }
+
+   for (i = 0; i  eccsteps ; i++) {
+   eccflag = 0;/* initialize eccflag */
+
+   for (j = 0; (j  eccsize); j++) {
+   if (read_ecc[j] != 0xFF) {
+   eccflag = 1;/* data area is flashed 
*/
   
   Just a reminder: this way of checking if a page has been programmed is 
   not robust to bitflips,
   so you may get into trouble with UBIFS on a fairly recent device.
 
 Sorry I missed this point.
 
 Here we were checking data in spare area (only in ecc layout 14*4 for BCH8) 
 is 0xFF. If all data
 in spare area is 0xFF, conclude that the page is erased  no need of error 
 correction. In case
 of bit flip present in spare area, page will be reported as uncorrectable.
 But I am not understand understand  trouble with UBIFS on a fairly recent 
 device.
 Can you little elaborativ

There are at least 2 potential problems when reading an erased page with 
bitflips:

1. bitflip in data area and no bitflip in spare area (all 0xff)
Your code will not perform any ECC correction.
UBIFS does not like finding bitflips in empty pages, see for instance
http://lists.infradead.org/pipermail/linux-mtd/2012-March/040328.html.

2. bitflip in ECC bytes in spare area
Your code will report an uncorrectable error upon reading; if this happens 
while reading a partially programmed UBI block,
I guess you will lose data.

BR,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ARM: OMAP2: gpmc: Add support for BCH ECC scheme

2012-10-04 Thread Ivan Djelic 
On Thu, Oct 04, 2012 at 09:03:42AM +0100, Philip, Avinash wrote:
(...)
   +int gpmc_calculate_ecc_bch(int cs, const u_char *dat, u_char *ecc)
   +{
   + int i, eccbchtsel;
   + u32 nsectors, reg, bch_val1, bch_val2, bch_val3, bch_val4;
   +
   + if (gpmc_ecc_used != cs)
   + return -EINVAL;
   +
   + /* read number of sectors for ecc to be calculated */
   + nsectors = ((gpmc_read_reg(GPMC_ECC_CONFIG)  4)  0x7) + 1;
   + /*
   +  * find BCH scheme used
   +  * 0 - BCH4
   +  * 1 - BCH8
   +  */
   + eccbchtsel = ((gpmc_read_reg(GPMC_ECC_CONFIG)  12)  0x3);
   +
   + /* update ecc bytes for entire page */
   + for (i = 0; i  nsectors; i++) {
   +
   + reg = GPMC_ECC_BCH_RESULT_0 + 16 * i;
   +
   + /* Read hw-computed remainder */
   + bch_val1 = gpmc_read_reg(reg + 0);
   + bch_val2 = gpmc_read_reg(reg + 4);
   + if (eccbchtsel) {
   + bch_val3 = gpmc_read_reg(reg + 8);
   + bch_val4 = gpmc_read_reg(reg + 12);
   + }
   +
   + if (eccbchtsel) {
   + /* BCH8 ecc scheme */
   + *ecc++ = (bch_val4  0xFF);
   + *ecc++ = ((bch_val3  24)  0xFF);
   + *ecc++ = ((bch_val3  16)  0xFF);
   + *ecc++ = ((bch_val3  8)  0xFF);
   + *ecc++ = (bch_val3  0xFF);
   + *ecc++ = ((bch_val2  24)  0xFF);
   + *ecc++ = ((bch_val2  16)  0xFF);
   + *ecc++ = ((bch_val2  8)  0xFF);
   + *ecc++ = (bch_val2  0xFF);
   + *ecc++ = ((bch_val1  24)  0xFF);
   + *ecc++ = ((bch_val1  16)  0xFF);
   + *ecc++ = ((bch_val1  8)  0xFF);
   + *ecc++ = (bch_val1  0xFF);
   + /* 14th byte of ecc not used */
   + *ecc++ = 0;
   + } else {
   + /* BCH4 ecc scheme */
   + *ecc++ = ((bch_val2  12)  0xFF);
   + *ecc++ = ((bch_val2  4)  0xFF);
   + *ecc++ = (((bch_val2  0xF)  4) |
   + ((bch_val1  28)  0xF));
   + *ecc++ = ((bch_val1  20)  0xFF);
   + *ecc++ = ((bch_val1  12)  0xFF);
   + *ecc++ = ((bch_val1  4)  0xFF);
   + *ecc++ = ((bch_val1  0xF)  4);
   + }
   + }
   +
   + gpmc_ecc_used = -EINVAL;
   + return 0;
   +}
   +EXPORT_SYMBOL_GPL(gpmc_calculate_ecc_bch);
  
  Here you introduce a function very similar to gpmc_calculate_ecc_bch4 and
  gpmc_calculate_ecc_bch8, but without the added benefit (IMHO) of the 
  constant
  polynomial that allows to get an ecc sequence of 0xFFs for a buffer filled 
  with
  0xFFs. Why ?
 
 I don't exactly understand what we benefitted/achieve. In my observation,
 this API does spare area also written with 0xFF if data area is 0xFFs.
 So the area looks like erased page again.

Precisely. It means you can read the page with ECC enabled without having to 
check if
the page has been programmed; it also enables bitflip correction on erased 
pages.

  If using the ELM prevents you from reusing gpmc_calculate_ecc_bch[48], 
  could you explain in which way ?
 
 When using gpmc_calculate_ecc_bch[48], calculated ecc values modified.
 The read sequence we following is
 Read 512 byte - read ECC bytes from spare area
 Now the calculated ECC will be zero if no error is reported. In case of 
 error, a syndrome
 Polynomial is reported. In either case modifying will corrupt the data.

It is still possible to retrieve your original error syndrome, even using the 
technique transforming ECC on erased pages into 0xFFs.
But I guess you're not interested if you need RBL compatibility.

BR,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ARM: OMAP2: gpmc: Add support for BCH ECC scheme

2012-10-03 Thread Ivan Djelic 
On Wed, Oct 03, 2012 at 03:29:48PM +0100, Philip, Avinash wrote:
 Add support for BCH ECC scheme to gpmc driver and also enabling multi
 sector read/write. This helps in doing single shot NAND page read and
 write.
 
 ECC engine configurations
 BCH 4 bit support
 1. write = ECC engine configured in wrap mode 6 and with ecc_size0 as 32.
 2. read  = ECC engine configured in wrap mode 1 and with ecc_size0 as
 13 and ecc_size1 as 1.
 
 BCH 8 bit support
 1. write = ECC engine configured in wrap mode 6 and with ecc_size0 as 32.
 2. read  = ECC engine configured in wrap mode 1 and with ecc_size0 as
 26 and ecc_size1 as 2.
 
 Note: For BCH8 ECC bytes set to 14 to make compatible with RBL.
 

Hi Philip,

I have a few comments/questions below,

(...)
 diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
 index 72428bd..c9bc3cf 100644
 --- a/arch/arm/mach-omap2/gpmc.c
 +++ b/arch/arm/mach-omap2/gpmc.c
 @@ -24,6 +24,7 @@
  #include linux/io.h
  #include linux/module.h
  #include linux/interrupt.h
 +#include linux/mtd/nand.h
  
  #include asm/mach-types.h
  #include plat/gpmc.h
 @@ -83,6 +84,18 @@
  #define ENABLE_PREFETCH  (0x1  7)
  #define DMA_MPU_MODE 2
  
 +/* GPMC ecc engine settings for read */
 +#define BCH_WRAPMODE_1   1   /* BCH wrap mode 6 */

Comment should say mode 1.

(...)
  /**
 + * gpmc_calculate_ecc_bch- Generate ecc bytes per block of 512 data 
 bytes for entire page
 + * @cs:  chip select number
 + * @dat: The pointer to data on which ECC is computed
 + * @ecc: The ECC output buffer
 + */
 +int gpmc_calculate_ecc_bch(int cs, const u_char *dat, u_char *ecc)
 +{
 + int i, eccbchtsel;
 + u32 nsectors, reg, bch_val1, bch_val2, bch_val3, bch_val4;
 +
 + if (gpmc_ecc_used != cs)
 + return -EINVAL;
 +
 + /* read number of sectors for ecc to be calculated */
 + nsectors = ((gpmc_read_reg(GPMC_ECC_CONFIG)  4)  0x7) + 1;
 + /*
 +  * find BCH scheme used
 +  * 0 - BCH4
 +  * 1 - BCH8
 +  */
 + eccbchtsel = ((gpmc_read_reg(GPMC_ECC_CONFIG)  12)  0x3);
 +
 + /* update ecc bytes for entire page */
 + for (i = 0; i  nsectors; i++) {
 +
 + reg = GPMC_ECC_BCH_RESULT_0 + 16 * i;
 +
 + /* Read hw-computed remainder */
 + bch_val1 = gpmc_read_reg(reg + 0);
 + bch_val2 = gpmc_read_reg(reg + 4);
 + if (eccbchtsel) {
 + bch_val3 = gpmc_read_reg(reg + 8);
 + bch_val4 = gpmc_read_reg(reg + 12);
 + }
 +
 + if (eccbchtsel) {
 + /* BCH8 ecc scheme */
 + *ecc++ = (bch_val4  0xFF);
 + *ecc++ = ((bch_val3  24)  0xFF);
 + *ecc++ = ((bch_val3  16)  0xFF);
 + *ecc++ = ((bch_val3  8)  0xFF);
 + *ecc++ = (bch_val3  0xFF);
 + *ecc++ = ((bch_val2  24)  0xFF);
 + *ecc++ = ((bch_val2  16)  0xFF);
 + *ecc++ = ((bch_val2  8)  0xFF);
 + *ecc++ = (bch_val2  0xFF);
 + *ecc++ = ((bch_val1  24)  0xFF);
 + *ecc++ = ((bch_val1  16)  0xFF);
 + *ecc++ = ((bch_val1  8)  0xFF);
 + *ecc++ = (bch_val1  0xFF);
 + /* 14th byte of ecc not used */
 + *ecc++ = 0;
 + } else {
 + /* BCH4 ecc scheme */
 + *ecc++ = ((bch_val2  12)  0xFF);
 + *ecc++ = ((bch_val2  4)  0xFF);
 + *ecc++ = (((bch_val2  0xF)  4) |
 + ((bch_val1  28)  0xF));
 + *ecc++ = ((bch_val1  20)  0xFF);
 + *ecc++ = ((bch_val1  12)  0xFF);
 + *ecc++ = ((bch_val1  4)  0xFF);
 + *ecc++ = ((bch_val1  0xF)  4);
 + }
 + }
 +
 + gpmc_ecc_used = -EINVAL;
 + return 0;
 +}
 +EXPORT_SYMBOL_GPL(gpmc_calculate_ecc_bch);

Here you introduce a function very similar to gpmc_calculate_ecc_bch4 and
gpmc_calculate_ecc_bch8, but without the added benefit (IMHO) of the constant
polynomial that allows to get an ecc sequence of 0xFFs for a buffer filled with
0xFFs. Why ?
If using the ELM prevents you from reusing gpmc_calculate_ecc_bch[48], could 
you explain in which way ?

Best regards,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] mtd: nand: omap2: Add data correction support

2012-10-03 Thread Ivan Djelic 
On Wed, Oct 03, 2012 at 03:29:49PM +0100, Philip, Avinash wrote:
 ELM module can be used for error correction of BCH 4  8 bit. Also
 support read  write page in one shot by adding custom read_page 
 write_page methods. This helps in optimizing code.
 
 New structure member is_elm_used is added to know the status of
 whether the ELM module is used for error correction or not.
 
 Note:
 ECC layout of BCH8 uses 14 bytes for 512 byte of data to make compatible
 with RBL ECC layout, even though the requirement was only 13 byte. This
 results a common ecc layout across RBL, U-boot  Linux.
 

See a few comments below,

(...)
 +static int omap_elm_correct_data(struct mtd_info *mtd, u_char *dat,
 +   u_char *read_ecc, u_char *calc_ecc)
 +{
 +   struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
 +   mtd);
 +   int eccsteps = info-nand.ecc.steps;
 +   int i , j, stat = 0;
 +   int eccsize, eccflag, size;
 +   struct elm_errorvec err_vec[ERROR_VECTOR_MAX];
 +   u_char *ecc_vec = calc_ecc;
 +   enum bch_ecc type;
 +   bool is_error_reported = false;
 +
 +   /* initialize elm error vector to zero */
 +   memset(err_vec, 0, sizeof(err_vec));
 +   if (info-nand.ecc.strength == BCH8_MAX_ERROR) {
 +   size = BCH8_SIZE;
 +   eccsize = BCH8_ECC_OOB_BYTES;
 +   type = BCH8_ECC;
 +   } else {
 +   size = BCH4_SIZE;
 +   eccsize = BCH4_SIZE;
 +   type = BCH4_ECC;
 +   }
 +
 +   for (i = 0; i  eccsteps ; i++) {
 +   eccflag = 0;/* initialize eccflag */
 +
 +   for (j = 0; (j  eccsize); j++) {
 +   if (read_ecc[j] != 0xFF) {
 +   eccflag = 1;/* data area is flashed */

Just a reminder: this way of checking if a page has been programmed is not 
robust to bitflips,
so you may get into trouble with UBIFS on a fairly recent device.

(...)
 @@ -1039,14 +1180,45 @@ static void omap3_enable_hwecc_bch(struct mtd_info 
 *mtd, int mode)
 
 nerrors = info-nand.ecc.strength;
 dev_width = (chip-options  NAND_BUSWIDTH_16) ? 1 : 0;
 +#ifdef CONFIG_MTD_NAND_OMAP_BCH
 +   if (info-is_elm_used) {
 +   /*
 +* Program GPMC to perform correction on (steps * 512) byte
 +* sector at a time.
 +*/
 +   gpmc_enable_hwecc_bch(info-gpmc_cs, mode, dev_width,
 +   info-nand.ecc.steps, nerrors);
 +   return;
 +   }
 +#endif
 /*
 -* Program GPMC to perform correction on one 512-byte sector at a 
 time.
 -* Using 4 sectors at a time (i.e. ecc.size = 2048) is also possible 
 and
 -* gives a slight (5%) performance gain (but requires additional 
 code).
 +* Program GPMC to perform correction on one 512-byte sector at
 +* a time.

Why removing the comment about 4-sector perf gain ? :-)

(...)
 @@ -1146,35 +1402,62 @@ static int omap3_init_bch(struct mtd_info *mtd, int 
 ecc_opt)
 goto fail;
 }
 
 -   /* initialize GPMC BCH engine */
 -   ret = gpmc_init_hwecc_bch(info-gpmc_cs, 1, max_errors);
 -   if (ret)
 -   goto fail;
 -
 -   /* software bch library is only used to detect and locate errors */
 -   info-bch = init_bch(13, max_errors, 0x201b /* hw polynomial */);
 -   if (!info-bch)
 -   goto fail;
 +   info-nand.ecc.size = 512;
 +   info-nand.ecc.hwctl = omap3_enable_hwecc_bch;
 +   info-nand.ecc.mode = NAND_ECC_HW;
 +   info-nand.ecc.strength = hw_errors;
 
 -   info-nand.ecc.size= 512;
 -   info-nand.ecc.hwctl   = omap3_enable_hwecc_bch;
 -   info-nand.ecc.correct = omap3_correct_data_bch;
 -   info-nand.ecc.mode= NAND_ECC_HW;
 +   if (info-is_elm_used  (mtd-writesize = 4096)) {
 +   enum bch_ecc bch_type;
 
 -   /*
 -* The number of corrected errors in an ecc block that will trigger
 -* block scrubbing defaults to the ecc strength (4 or 8).
 -* Set mtd-bitflip_threshold here to define a custom threshold.
 -*/
 +   if (hw_errors == BCH8_MAX_ERROR) {
 +   bch_type = BCH8_ECC;
 +   info-nand.ecc.bytes = BCH8_SIZE;
 +   } else {
 +   bch_type = BCH4_ECC;
 +   info-nand.ecc.bytes = BCH4_SIZE;
 +   }
 
 -   if (max_errors == 8) {
 -   info-nand.ecc.strength  = 8;
 -   info-nand.ecc.bytes = 13;
 -   info-nand.ecc.calculate = omap3_calculate_ecc_bch8;
 +   info-nand.ecc.correct = omap_elm_correct_data;
 +   info-nand.ecc.calculate = omap3_calculate_ecc_bch;
 +   info-nand.ecc.read_page = omap_read_page_bch;
 +   info-nand.ecc.write_page =

Re: [PATCH 0/3] GPMC NAND isr using standard API

2012-05-19 Thread Ivan Djelic 
On Tue, May 15, 2012 at 03:38:09PM +0100, Afzal Mohammed wrote:
 Hi Tony, Artem,
 
 This series creates a fictitious GPMC interrupt chip and provide the
 clients with interrupts that could be handled using standard APIs.
 This helps in removing the requirement of driver of peripheral
 connected to GPMC having the knowledge about GPMC. The only user is
 OMAP NAND driver, it has also been modified to use interrupts provided
 by imaginary GPMC chip.
 
 This series has a dependency on [2], while [2] has a trivial
 dependency on [1].
 
 With this series plus [1,2], GPMC driver conversion which is going to
 happen shortly will not create noticable effect outside of
 arch/arm/*omap*/.
 
 If this series along with [1,2] can be taken in for 3.5, ripples felt
 by MTD drivers upon GPMC driver conversion would be minimal.

Hi Afzal,

I tried to take your series of patches, but I had issues with the
first [1] (I did not try the others): it depends on the following patch,
which is not in the l2-mtd-2.6 tree:

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg68258.html

and it does not apply anyway to l2-mtd-2.6 because of (at least) the
following patches:

http://lists.infradead.org/pipermail/linux-mtd/2012-April/040631.html
http://lists.infradead.org/pipermail/linux-mtd/2012-April/040724.html

So, do you think you could rebase your series on l2-mtd-2.6 ?
And maybe merge the 3 series into a single one, if they have circular
dependencies ?

Thanks,

--
Ivan

[1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg68581.html
[2] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg68652.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] ARM: OMAP2+: gpmc: update nand register helper

2012-05-15 Thread Ivan Djelic 
On Fri, May 11, 2012 at 09:02:14PM +0100, Tony Lindgren wrote:
 * Afzal Mohammed af...@ti.com [120511 08:48]:
  Provide helper function for updating NAND register details for
  the necessary chip select. NAND drivers platform data can be
  updated with this information so that NAND driver can handle
  GPMC NAND operations by itself.
 
 Hmm this seems that it might be a more future proof path.

OK, I'll try to rewrite my patch on top of these.

Best Regards,
--
Ivan

 
 Tony
 
  
  Signed-off-by: Afzal Mohammed af...@ti.com
  ---
   arch/arm/mach-omap2/gpmc.c |   21 +
   arch/arm/plat-omap/include/plat/gpmc.h |   18 ++
   2 files changed, 39 insertions(+)
  
  diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
  index 46b09da..a409a3e 100644
  --- a/arch/arm/mach-omap2/gpmc.c
  +++ b/arch/arm/mach-omap2/gpmc.c
  @@ -49,6 +49,7 @@
   #define GPMC_ECC_CONTROL   0x1f8
   #define GPMC_ECC_SIZE_CONFIG   0x1fc
   #define GPMC_ECC1_RESULT0x200
  +#define GPMC_ECC_BCH_RESULT_0  0x240
   
   /* GPMC ECC control settings */
   #define GPMC_ECC_CTRL_ECCCLEAR 0x100
  @@ -681,6 +682,26 @@ int gpmc_prefetch_reset(int cs)
   }
   EXPORT_SYMBOL(gpmc_prefetch_reset);
   
  +void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs)
  +{
  +   reg-gpmc_status = gpmc_base + GPMC_STATUS;
  +   reg-gpmc_nand_command = gpmc_base + GPMC_CS0_OFFSET +
  +   GPMC_CS_NAND_COMMAND + GPMC_CS_SIZE * cs;
  +   reg-gpmc_nand_address = gpmc_base + GPMC_CS0_OFFSET +
  +   GPMC_CS_NAND_ADDRESS + GPMC_CS_SIZE * cs;
  +   reg-gpmc_nand_data = gpmc_base + GPMC_CS0_OFFSET +
  +   GPMC_CS_NAND_DATA + GPMC_CS_SIZE * cs;
  +   reg-gpmc_prefetch_config1 = gpmc_base + GPMC_PREFETCH_CONFIG1;
  +   reg-gpmc_prefetch_config2 = gpmc_base + GPMC_PREFETCH_CONFIG2;
  +   reg-gpmc_prefetch_control = gpmc_base + GPMC_PREFETCH_CONTROL;
  +   reg-gpmc_prefetch_status = gpmc_base + GPMC_PREFETCH_STATUS;
  +   reg-gpmc_ecc_config = gpmc_base + GPMC_ECC_CONFIG;
  +   reg-gpmc_ecc_control = gpmc_base + GPMC_ECC_CONTROL;
  +   reg-gpmc_ecc_size_config = gpmc_base + GPMC_ECC_SIZE_CONFIG;
  +   reg-gpmc_ecc1_result = gpmc_base + GPMC_ECC1_RESULT;
  +   reg-gpmc_bch_result0 = gpmc_base + GPMC_ECC_BCH_RESULT_0;
  +}
  +
   static void __init gpmc_mem_init(void)
   {
  int cs;
  diff --git a/arch/arm/plat-omap/include/plat/gpmc.h 
  b/arch/arm/plat-omap/include/plat/gpmc.h
  index 1527929..6a8078e 100644
  --- a/arch/arm/plat-omap/include/plat/gpmc.h
  +++ b/arch/arm/plat-omap/include/plat/gpmc.h
  @@ -131,6 +131,24 @@ struct gpmc_timings {
  u16 wr_data_mux_bus;/* WRDATAONADMUXBUS */
   };
   
  +struct gpmc_nand_regs {
  +   void __iomem*gpmc_status;
  +   void __iomem*gpmc_nand_command;
  +   void __iomem*gpmc_nand_address;
  +   void __iomem*gpmc_nand_data;
  +   void __iomem*gpmc_prefetch_config1;
  +   void __iomem*gpmc_prefetch_config2;
  +   void __iomem*gpmc_prefetch_control;
  +   void __iomem*gpmc_prefetch_status;
  +   void __iomem*gpmc_ecc_config;
  +   void __iomem*gpmc_ecc_control;
  +   void __iomem*gpmc_ecc_size_config;
  +   void __iomem*gpmc_ecc1_result;
  +   void __iomem*gpmc_bch_result0;
  +};
  +
  +extern void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs);
  +
   extern unsigned int gpmc_ns_to_ticks(unsigned int time_ns);
   extern unsigned int gpmc_ps_to_ticks(unsigned int time_ps);
   extern unsigned int gpmc_ticks_to_ns(unsigned int ticks);
  -- 
  1.7.10
  

-- 
Ivan Djelic
Operating System Team Manager
tel + 33 01 48 03 70 16
-
Parrot
174, Quai de Jemmapes
75010 Paris, France
tel + 33 01 48 03 60 60
fax + 33 01 48 03 70 08
-
http://www.parrot.com
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] ARM: OMAP3: gpmc: add BCH ecc api and modes

2012-05-10 Thread Ivan Djelic 
On Thu, May 10, 2012 at 04:52:18PM +0100, Artem Bityutskiy wrote:
 On Thu, 2012-05-10 at 17:17 +0200, Ivan Djelic wrote:
  But in order to do so, I need the changes that Afzal has submitted
  (in particular [3]). Those changes (and as a consequence, my new patch)
  won't hit 3.5.
  
  So, when Afzal's patches are pushed, I'll submit a new, single MTD patch.
 
 But this is not going to happen this merge window as I understood, may
 be not even the next one. We need to make UBIFS happy sooner than that,
 I think. So may be we go forward with your original patch?

I'm OK with this too, as the patches are ready and tested.
The MTD patch is [2], it depends on [1] which has been pushed, then dropped by 
Tony.
Do you need me to repost [2] ?

Tony, sorry to backpedal on this: would you re-push patch [1], if indeed 
Afzal's patches
are not going to be merged soon ? In the meantime, I can prepare a patch on top 
of Afzal's to
have a smooth transition w.r.t BCH support. What do you think ?

Best Regards,
--
Ivan

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-April/040965.html
[2] http://lists.infradead.org/pipermail/linux-mtd/2012-April/041020.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] ARM: OMAP3: gpmc: add BCH ecc api and modes

2012-05-09 Thread Ivan Djelic 
On Wed, May 09, 2012 at 01:29:28AM +0100, Tony Lindgren wrote:
 * Ivan Djelic ivan.dje...@parrot.com [120426 05:23]:
  Hello,
  
  Here is version 3 of this patch after review from Tony Lindgren.
  This version adds a separate initialization function mostly to check CPU
  compatibility. This check cannot be done in gpmc_enable_hwecc_bch() (which
  is meant to be called from mtd function ecc.hwctl) because ecc.hwctl is
  not called before the first NAND read access, and it cannot return an error
  status.
 
 Thanks applying into devel-gpmc branch.

OK thanks!

I still have a question though: there are recent patches from
Afzal Mohammed that seem to go into the opposite direction, that is 
giving back GPMC register access to the omap2 NAND driver.
In particular, [PATCH v4 17/39] [1] commit message says:

  GPMC driver has been modified to fill NAND platform data with GPMC
  NAND register details. As these registers are accessible in NAND
  driver itself, configure NAND in GPMC by itself.

This also includes ecc configuration. My original mtd driver patch indeed had
ecc handling code inside the driver (not in arch/arm/mach-omap2/gpmc.c).

So, my question is: which direction are we going to with respect to this
OMAP GPMC/NAND code separation ?

Note that I could prepare a new MTD patch with BCH ecc code included,
allowing to drop the GPMC BCH ecc api.

BR,
--
Ivan

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-May/041105.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] ARM: OMAP3: gpmc: add BCH ecc api and modes

2012-05-09 Thread Ivan Djelic 
On Wed, May 09, 2012 at 01:29:28AM +0100, Tony Lindgren wrote:
 * Ivan Djelic ivan.dje...@parrot.com [120426 05:23]:
  Hello,
  
  Here is version 3 of this patch after review from Tony Lindgren.
  This version adds a separate initialization function mostly to check CPU
  compatibility. This check cannot be done in gpmc_enable_hwecc_bch() (which
  is meant to be called from mtd function ecc.hwctl) because ecc.hwctl is
  not called before the first NAND read access, and it cannot return an error
  status.
 
 Thanks applying into devel-gpmc branch.

OK thanks!

I still have a question though: there are recent patches from
Afzal Mohammed that seem to go into the opposite direction, that is
giving back GPMC register access to the omap2 NAND driver.
In particular, [PATCH v4 17/39] [1] commit message says:

  GPMC driver has been modified to fill NAND platform data with GPMC
  NAND register details. As these registers are accessible in NAND
  driver itself, configure NAND in GPMC by itself.

This also includes ecc configuration. My original mtd driver patch indeed had
ecc handling code inside the driver (not in arch/arm/mach-omap2/gpmc.c).

So, my question is: which direction are we going to with respect to this
OMAP GPMC/NAND code separation ?

Note that I could prepare a new MTD patch with BCH ecc code included,
allowing to drop the GPMC BCH ecc api.

BR,
--
Ivan

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-May/041105.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] mtd: nand: omap: add support for hardware BCH ecc

2012-04-30 Thread Ivan Djelic 
Hello,
This patch provides hardware NAND BCH ecc support for OMAP3 boards.
It depends on the following patch:

new GPMC BCH api v3 (linux-omap):
http://lists.infradead.org/pipermail/linux-mtd/2012-April/040965.html

This v2 leverages on Mike Dunn's MTD bitflip threshold feature
(5568eb3) in order to drop error concealment code. It also uses the
updated GPMC BCH api provided in aforementioned patch.

--
Ivan

Two modes are supported: 4-bit and 8-bit error correction.
Note that 4-bit mode is only confirmed to work on OMAP3630 ES 1.x,
x = 1. The OMAP3 GPMC hardware BCH engine computes remainder
polynomials, it does not provide automatic error location and
correction: this step is implemented using the BCH library.

This implementation only protects page data, there is no support
for protecting user-defined spare area bytes (this could be added
with few modifications); therefore, it cannot be used with YAFFS2
or other similar filesystems that depend on oob storage.

Before being stored to nand flash, hardware BCH ecc is adjusted
so that an erased page has a valid ecc; thus allowing correction of
bitflips in blank pages (also common on 4-bit devices).

BCH correction mode is selected at runtime by setting platform data
parameter 'ecc_opt' to value OMAP_ECC_BCH4_CODE_HW or
OMAP_ECC_BCH8_CODE_HW.

This code has been tested with mtd test modules, UBI and UBIFS on a
BeagleBoard revC3 (OMAP3530 ES3.0 + Micron NAND 256MiB 1,8V 16-bit).

Signed-off-by: Ivan Djelic ivan.dje...@parrot.com
---
 v2 changelog:
 - drop error concealment code in favour of generic MTD support
 - use GMPC BCH api init to check CPU compatibility

 drivers/mtd/nand/Kconfig |   40 
 drivers/mtd/nand/omap2.c |  245 ++
 2 files changed, 285 insertions(+)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 7d17cec..da61f85 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -115,6 +115,46 @@ config MTD_NAND_OMAP2
   Support for NAND flash on Texas Instruments OMAP2, OMAP3 and OMAP4
  platforms.
 
+config MTD_NAND_OMAP_BCH
+   depends on MTD_NAND  MTD_NAND_OMAP2  ARCH_OMAP3
+   bool Enable support for hardware BCH error correction
+   default n
+   select BCH
+   select BCH_CONST_PARAMS
+   help
+Support for hardware BCH error correction.
+
+choice
+   prompt BCH error correction capability
+   depends on MTD_NAND_OMAP_BCH
+
+config MTD_NAND_OMAP_BCH8
+   bool 8 bits / 512 bytes (recommended)
+   help
+Support correcting up to 8 bitflips per 512-byte block.
+This will use 13 bytes of spare area per 512 bytes of page data.
+This is the recommended mode, as 4-bit mode does not work
+on some OMAP3 revisions, due to a hardware bug.
+
+config MTD_NAND_OMAP_BCH4
+   bool 4 bits / 512 bytes
+   help
+Support correcting up to 4 bitflips per 512-byte block.
+This will use 7 bytes of spare area per 512 bytes of page data.
+Note that this mode does not work on some OMAP3 revisions, due to a
+hardware bug. Please check your OMAP datasheet before selecting this
+mode.
+
+endchoice
+
+if MTD_NAND_OMAP_BCH
+config BCH_CONST_M
+   default 13
+config BCH_CONST_T
+   default 4 if MTD_NAND_OMAP_BCH4
+   default 8 if MTD_NAND_OMAP_BCH8
+endif
+
 config MTD_NAND_IDS
tristate
 
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 05d3562..d7f681d 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -21,6 +21,10 @@
 #include linux/io.h
 #include linux/slab.h
 
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+#include linux/bch.h
+#endif
+
 #include plat/dma.h
 #include plat/gpmc.h
 #include plat/nand.h
@@ -127,6 +131,11 @@ struct omap_nand_info {
} iomode;
u_char  *buf;
int buf_len;
+
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+   struct bch_control *bch;
+   struct nand_ecclayout   ecclayout;
+#endif
 };
 
 /**
@@ -929,6 +938,226 @@ static int omap_dev_ready(struct mtd_info *mtd)
return 1;
 }
 
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+
+/**
+ * omap3_enable_hwecc_bch - Program OMAP3 GPMC to perform BCH ECC correction
+ * @mtd: MTD device structure
+ * @mode: Read/Write mode
+ */
+static void omap3_enable_hwecc_bch(struct mtd_info *mtd, int mode)
+{
+   int nerrors;
+   unsigned int dev_width;
+   struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
+  mtd);
+   struct nand_chip *chip = mtd-priv;
+
+   nerrors = (info-nand.ecc.bytes == 13) ? 8 : 4;
+   dev_width = (chip-options  NAND_BUSWIDTH_16) ? 1 : 0;
+   /*
+* Program GPMC to perform correction on one 512-byte sector at a time.
+* Using 4 sectors at a time (i.e. ecc.size = 2048) is also possible and
+* gives a slight (5

[PATCH v3] ARM: OMAP3: gpmc: add BCH ecc api and modes

2012-04-26 Thread Ivan Djelic 
Hello,

Here is version 3 of this patch after review from Tony Lindgren.
This version adds a separate initialization function mostly to check CPU
compatibility. This check cannot be done in gpmc_enable_hwecc_bch() (which
is meant to be called from mtd function ecc.hwctl) because ecc.hwctl is
not called before the first NAND read access, and it cannot return an error
status.

--
Ivan

This patch adds a simple BCH ecc computation api, similar to the
existing Hamming ecc api. It is intended to be used by the MTD layer.
It implements the following features:

- support 4-bit and 8-bit ecc computation
- do not protect user bytes in spare area, only data area is protected
- ecc for an erased NAND page (0xFFs) is also a sequence of 0xFFs

This last feature is obtained by adding a constant polynomial to
the hardware computed ecc. It allows to correct bitflips in blank pages
and is extremely useful to support filesystems such as UBIFS, which expect
erased pages to contain only 0xFFs.

This api has been tested on an OMAP3630 board.

Signed-off-by: Ivan Djelic ivan.dje...@parrot.com
---
 v3 changelog: added init function to check CPU compatibility
 v2 changelog: added missing control register configuration

 arch/arm/mach-omap2/gpmc.c |  184 
 arch/arm/plat-omap/include/plat/gpmc.h |   11 ++
 2 files changed, 195 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 00d5108..1ca8d7f 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -49,6 +49,7 @@
 #define GPMC_ECC_CONTROL   0x1f8
 #define GPMC_ECC_SIZE_CONFIG   0x1fc
 #define GPMC_ECC1_RESULT0x200
+#define GPMC_ECC_BCH_RESULT_0   0x240   /* not available on OMAP2 */
 
 #define GPMC_CS0_OFFSET0x60
 #define GPMC_CS_SIZE   0x30
@@ -920,3 +921,186 @@ int gpmc_calculate_ecc(int cs, const u_char *dat, u_char 
*ecc_code)
return 0;
 }
 EXPORT_SYMBOL_GPL(gpmc_calculate_ecc);
+
+#ifdef CONFIG_ARCH_OMAP3
+
+/**
+ * gpmc_init_hwecc_bch - initialize hardware BCH ecc functionality
+ * @cs: chip select number
+ * @nsectors: how many 512-byte sectors to process
+ * @nerrors: how many errors to correct per sector (4 or 8)
+ *
+ * This function must be executed before any call to gpmc_enable_hwecc_bch.
+ */
+int gpmc_init_hwecc_bch(int cs, int nsectors, int nerrors)
+{
+   /* check if ecc module is in use */
+   if (gpmc_ecc_used != -EINVAL)
+   return -EINVAL;
+
+   /* support only OMAP3 class */
+   if (!cpu_is_omap34xx()) {
+   printk(KERN_ERR BCH ecc is not supported on this CPU\n);
+   return -EINVAL;
+   }
+
+   /*
+* For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x=1.
+* Other chips may be added if confirmed to work.
+*/
+   if ((nerrors == 4) 
+   (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) {
+   printk(KERN_ERR BCH 4-bit mode is not supported on this 
CPU\n);
+   return -EINVAL;
+   }
+
+   /* sanity check */
+   if (nsectors  8) {
+   printk(KERN_ERR BCH cannot process %d sectors (max is 8)\n,
+  nsectors);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(gpmc_init_hwecc_bch);
+
+/**
+ * gpmc_enable_hwecc_bch - enable hardware BCH ecc functionality
+ * @cs: chip select number
+ * @mode: read/write mode
+ * @dev_width: device bus width(1 for x16, 0 for x8)
+ * @nsectors: how many 512-byte sectors to process
+ * @nerrors: how many errors to correct per sector (4 or 8)
+ */
+int gpmc_enable_hwecc_bch(int cs, int mode, int dev_width, int nsectors,
+ int nerrors)
+{
+   unsigned int val;
+
+   /* check if ecc module is in use */
+   if (gpmc_ecc_used != -EINVAL)
+   return -EINVAL;
+
+   gpmc_ecc_used = cs;
+
+   /* clear ecc and enable bits */
+   gpmc_write_reg(GPMC_ECC_CONTROL, 0x1);
+
+   /*
+* When using BCH, sector size is hardcoded to 512 bytes.
+* Here we are using wrapping mode 6 both for reading and writing, with:
+*  size0 = 0  (no additional protected byte in spare area)
+*  size1 = 32 (skip 32 nibbles = 16 bytes per sector in spare area)
+*/
+   gpmc_write_reg(GPMC_ECC_SIZE_CONFIG, (32  22) | (0  12));
+
+   /* BCH configuration */
+   val = ((1 16) | /* enable BCH */
+  (((nerrors == 8) ? 1 : 0)  12) | /* 8 or 4 bits */
+  (0x06   8) | /* wrap mode = 6 */
+  (dev_width  7) | /* bus width */
+  (((nsectors-1)  0x7)   4) | /* number of sectors */
+  (cs 1) | /* ECC CS */
+  (0x1));/* enable ECC */
+
+   gpmc_write_reg(GPMC_ECC_CONFIG, val);
+   gpmc_write_reg(GPMC_ECC_CONTROL, 0x101

Re: [PATCH v2] ARM: OMAP3: gpmc: add BCH ecc api and modes

2012-04-25 Thread Ivan Djelic 
Hi Tony,
Thanks for the review,

On Wed, Apr 25, 2012 at 06:03:14PM +0100, Tony Lindgren wrote:
(...)
   #define GPMC_ECC1_RESULT0x200
  +#define GPMC_ECC_BCH_RESULT_0   0x240
 
 Can you please add a comment here saying something like:
 
 #define GPMC_ECC_BCH_RESULT_0   0x240 /* Not available on omap2 */

OK sure.

  +   /* check if ecc module is in use */
  +   if (gpmc_ecc_used != -EINVAL)
  +   return -EINVAL;
  +   /*
  +* FIXME: some OMAP3 revisions have a hardware bug which prevents
  +* the 4-bit BCH mode from working properly. Such revisions could be
  +* detected and rejected here.
  +*/
 
 This should then be disabled to avoid corruption. Maybe only allow it
 initially on omaps that have been tested? And for omap2 it should return
 error  for sure.

OK I'll add a check.

 
 Or do you know the broken omap3 versions?

Well, I was hoping that someone from linux-omap could tell me :)
I found this HW ECC feature table in
http://processors.wiki.ti.com/index.php/Raw_NAND_ECC:

 1b 4b  8b
---
OMAP35x  YESNO  YES
AM35xYESYES YES
AM/DM37x YESYES YES

and other wiki pages confirmed that 4-bit mode is not supported on all OMAP35xx 
chips.
OTOH, I know from TI support that 4-bit mode is at least supported on
OMAP3630 ES1.x (x = 1).

So, a conservative approach would be to reject 4-bit mode on all chips but
omap3630 with rev = 1.1. Other revisions/chips could be added later if they are
confirmed to work; what do you think ?

 Also, should you first request this feature in case multiple drivers
 need to share it?

According to TI documentation (OMAP36xx ES1.x TRM, ยง10.1.4, GPMC functional 
diagram),
the GPMC ECC engines (Hamming and BCH) are dedicated to NAND access only; 
therefore
I believe the mtd driver is the only potential user of this feature.
Also, the existing Hamming ecc API does not perform any request; or did I miss
something? If I need to perform the request, is there an existing api to do so?

Thanks,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mtd: nand: omap: add support for hardware BCH ecc

2012-04-20 Thread Ivan Djelic 
Hello,
This patch provides hardware NAND BCH ecc support for OMAP3 boards.
It depends on the following patches:

new GPMC BCH api (linux-omap):
http://lists.infradead.org/pipermail/linux-mtd/2012-April/040757.html

race condition fix in OMAP mtd driver:
http://lists.infradead.org/pipermail/linux-mtd/2012-April/040724.html

I was able to run low-level mtd stress tests on a BeagleBoard, but I
would be interested if someone could test this patch with UBI/UBIFS.
BR,

Ivan

--
Two modes are supported: 4-bit and 8-bit error correction.
The OMAP3 GPMC hardware BCH engine computes remainder polynomials,
it does not provide automatic error location and correction: this
step is implemented using the BCH library.

Error concealment thresholds are implemented in order to avoid
over-scrubbing blocks with sticky bitflips (which are common on
4-bit nand devices).

This implementation only protects page data, there is no support
for protecting user-defined spare area bytes (this could be added
with a few modifications); therefore, it cannot be used with YAFFS2
or other similar filesystems that depend on oob storage.

Before being stored to nand flash, hardware BCH ecc is adjusted
so that an erased page has a valid ecc; thus allowing correction of
bitflips in blank pages (also common on 4-bit devices).

BCH correction mode is selected at runtime by setting platform data
parameter 'ecc_opt' to value OMAP_ECC_BCH4_CODE_HW or
OMAP_ECC_BCH8_CODE_HW.

This code has been tested on a BeagleBoard revC3
(OMAP3530 ES3.0 + Micron NAND 256MiB 1,8V 16-bit).

Signed-off-by: Ivan Djelic ivan.dje...@parrot.com
---
 drivers/mtd/nand/Kconfig |   40 +++
 drivers/mtd/nand/omap2.c |  262 ++
 2 files changed, 302 insertions(+)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 0ea7084..0b4ce93 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -115,6 +115,46 @@ config MTD_NAND_OMAP2
   Support for NAND flash on Texas Instruments OMAP2, OMAP3 and OMAP4
  platforms.
 
+config MTD_NAND_OMAP_BCH
+   depends on MTD_NAND  MTD_NAND_OMAP2  ARCH_OMAP3
+   bool Enable support for hardware BCH error correction
+   default n
+   select BCH
+   select BCH_CONST_PARAMS
+   help
+Support for hardware BCH error correction.
+
+choice
+   prompt BCH error correction capability
+   depends on MTD_NAND_OMAP_BCH
+
+config MTD_NAND_OMAP_BCH8
+   bool 8 bits / 512 bytes (recommended)
+   help
+Support correcting up to 8 bitflips per 512-byte block.
+This will use 13 bytes of spare area per 512 bytes of page data.
+This is the recommended mode, as 4-bit mode does not work
+on some OMAP3 revisions, due to a hardware bug.
+
+config MTD_NAND_OMAP_BCH4
+   bool 4 bits / 512 bytes
+   help
+Support correcting up to 4 bitflips per 512-byte block.
+This will use 7 bytes of spare area per 512 bytes of page data.
+Note that this mode does not work on some OMAP3 revisions, due to a
+hardware bug. Please check your OMAP datasheet before selecting this
+mode.
+
+endchoice
+
+if MTD_NAND_OMAP_BCH
+config BCH_CONST_M
+   default 13
+config BCH_CONST_T
+   default 4 if MTD_NAND_OMAP_BCH4
+   default 8 if MTD_NAND_OMAP_BCH8
+endif
+
 config MTD_NAND_IDS
tristate
 
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 45c6205..d03ef5f 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -21,6 +21,10 @@
 #include linux/io.h
 #include linux/slab.h
 
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+#include linux/bch.h
+#endif
+
 #include plat/dma.h
 #include plat/gpmc.h
 #include plat/nand.h
@@ -127,6 +131,12 @@ struct omap_nand_info {
} iomode;
u_char  *buf;
int buf_len;
+
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+   struct bch_control *bch;
+   struct nand_ecclayout   ecclayout;
+   int error_threshold;
+#endif
 };
 
 /**
@@ -927,6 +937,236 @@ static int omap_dev_ready(struct mtd_info *mtd)
return 1;
 }
 
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+
+/**
+ * omap3_enable_hwecc_bch - Program OMAP3 GPMC to perform BCH ECC correction
+ * @mtd: MTD device structure
+ * @mode: Read/Write mode
+ */
+static void omap3_enable_hwecc_bch(struct mtd_info *mtd, int mode)
+{
+   int nerrors;
+   unsigned int dev_width;
+   struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
+  mtd);
+   struct nand_chip *chip = mtd-priv;
+
+   nerrors = (info-nand.ecc.bytes == 13) ? 8 : 4;
+   dev_width = (chip-options  NAND_BUSWIDTH_16) ? 1 : 0;
+   /*
+* Program GPMC to perform correction on one 512-byte sector at a time.
+* Using 4 sectors at a time (i.e. ecc.size = 2048) is also possible

Re: [PATCH] mtd: nand: omap: add support for hardware BCH ecc

2012-04-20 Thread Ivan Djelic 
On Fri, Apr 20, 2012 at 12:12:27PM +0100, Grazvydas Ignotas wrote:
 On Fri, Apr 20, 2012 at 12:48 PM, Ivan Djelic ivan.dje...@parrot.com wrote:
  Hello,
  This patch provides hardware NAND BCH ecc support for OMAP3 boards.
  It depends on the following patches:
 
  new GPMC BCH api (linux-omap):
  http://lists.infradead.org/pipermail/linux-mtd/2012-April/040757.html
 
  race condition fix in OMAP mtd driver:
  http://lists.infradead.org/pipermail/linux-mtd/2012-April/040724.html
 
 I don't have this one in my mailbox (looked up through the archives),
 so commenting about it here. What about just dropping omap_wait()
 instead? I think gpmc_nand_read(.. GPMC_NAND_DATA) is equivalent to
 ordinary data read, knowing that omap_wait() looks like a duplicate of
 generic nand_wait(), just without LED support.

Yes, I agree; I also comtemplated getting rid of omap_wait(), but I needed this
quick fix out of the way to submit my BCH patch. But you're right, and there
are plenty other things to improve in omap2.c (look at omap_compare_ecc() for
a good example).
BR,
--
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: OMAP3: gpmc: add BCH ecc api and modes

2012-04-19 Thread Ivan Djelic 
Hello,
Here is version 2 of this patch, fixing in bug discovered while testing
on a BeagleBoard rev C3 (OMAP3530 ES3.0 + Micron NAND 256MiB 1,8V 16-bit).
--
Ivan


This patch adds a simple BCH ecc computation api, similar to the
existing Hamming ecc api. It is intended to be used by the MTD layer.
It implements the following features:

- support 4-bit and 8-bit ecc computation
- do not protect user bytes in spare area, only data area is protected
- ecc for an erased NAND page (0xFFs) is also a sequence of 0xFFs

This last feature is obtained by adding a constant polynomial to
the hardware computed ecc. It allows to correct bitflips in blank pages
and is extremely useful to support filesystems such as UBIFS, which expect
erased pages to contain only 0xFFs.

This api has been tested on an OMAP3630 board.

Signed-off-by: Ivan Djelic ivan.dje...@parrot.com
---
 v2 changelog: added missing control register configuration

 arch/arm/mach-omap2/gpmc.c |  148 
 arch/arm/plat-omap/include/plat/gpmc.h |   10 +++
 2 files changed, 158 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 00d5108..e3a91a1 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -49,6 +49,7 @@
 #define GPMC_ECC_CONTROL   0x1f8
 #define GPMC_ECC_SIZE_CONFIG   0x1fc
 #define GPMC_ECC1_RESULT0x200
+#define GPMC_ECC_BCH_RESULT_0   0x240
 
 #define GPMC_CS0_OFFSET0x60
 #define GPMC_CS_SIZE   0x30
@@ -920,3 +921,150 @@ int gpmc_calculate_ecc(int cs, const u_char *dat, u_char 
*ecc_code)
return 0;
 }
 EXPORT_SYMBOL_GPL(gpmc_calculate_ecc);
+
+#ifdef CONFIG_ARCH_OMAP3
+
+/**
+ * gpmc_enable_hwecc_bch - enable hardware BCH ecc functionality
+ * @cs: chip select number
+ * @mode: read/write mode
+ * @dev_width: device bus width(1 for x16, 0 for x8)
+ * @nsectors: how many 512-byte sectors to process
+ * @nerrors: how many errors to correct per sector (4 or 8)
+ */
+int gpmc_enable_hwecc_bch(int cs, int mode, int dev_width, int nsectors,
+ int nerrors)
+{
+   unsigned int val;
+
+   /* check if ecc module is in use */
+   if (gpmc_ecc_used != -EINVAL)
+   return -EINVAL;
+   /*
+* FIXME: some OMAP3 revisions have a hardware bug which prevents
+* the 4-bit BCH mode from working properly. Such revisions could be
+* detected and rejected here.
+*/
+
+   gpmc_ecc_used = cs;
+
+   /* clear ecc and enable bits */
+   gpmc_write_reg(GPMC_ECC_CONTROL, 0x1);
+
+   /*
+* When using BCH, sector size is hardcoded to 512 bytes.
+* Here we are using wrapping mode 6 both for reading and writing, with:
+*  size0 = 0  (no additional protected byte in spare area)
+*  size1 = 32 (skip 32 nibbles = 16 bytes per sector in spare area)
+*/
+   gpmc_write_reg(GPMC_ECC_SIZE_CONFIG, (32  22) | (0  12));
+
+   /* BCH configuration */
+   val = ((1 16) | /* enable BCH */
+  (((nerrors == 8) ? 1 : 0)  12) | /* 8 or 4 bits */
+  (0x06   8) | /* wrap mode = 6 */
+  (dev_width  7) | /* bus width */
+  (((nsectors-1)  0x7)   4) | /* number of sectors */
+  (cs 1) | /* ECC CS */
+  (0x1));/* enable ECC */
+
+   gpmc_write_reg(GPMC_ECC_CONFIG, val);
+   gpmc_write_reg(GPMC_ECC_CONTROL, 0x101);
+   return 0;
+}
+EXPORT_SYMBOL_GPL(gpmc_enable_hwecc_bch);
+
+/**
+ * gpmc_calculate_ecc_bch4 - Generate 7 ecc bytes per sector of 512 data bytes
+ * @cs:  chip select number
+ * @dat: The pointer to data on which ecc is computed
+ * @ecc: The ecc output buffer
+ */
+int gpmc_calculate_ecc_bch4(int cs, const u_char *dat, u_char *ecc)
+{
+   int i;
+   unsigned long nsectors, reg, val1, val2;
+
+   if (gpmc_ecc_used != cs)
+   return -EINVAL;
+
+   nsectors = ((gpmc_read_reg(GPMC_ECC_CONFIG)  4)  0x7) + 1;
+
+   for (i = 0; i  nsectors; i++) {
+
+   reg = GPMC_ECC_BCH_RESULT_0 + 16*i;
+
+   /* Read hw-computed remainder */
+   val1 = gpmc_read_reg(reg + 0);
+   val2 = gpmc_read_reg(reg + 4);
+
+   /*
+* Add constant polynomial to remainder, in order to get an ecc
+* sequence of 0xFFs for a buffer filled with 0xFFs; and
+* left-justify the resulting polynomial.
+*/
+   *ecc++ = 0x28 ^ ((val2  12)  0xFF);
+   *ecc++ = 0x13 ^ ((val2   4)  0xFF);
+   *ecc++ = 0xcc ^ (((val2  0xF)  4)|((val1  28)  0xF));
+   *ecc++ = 0x39 ^ ((val1  20)  0xFF);
+   *ecc++ = 0x96 ^ ((val1  12)  0xFF);
+   *ecc++ = 0xac ^ ((val1  4)  0xFF);
+   *ecc++ = 0x7f ^ ((val1  0xF)  4

[PATCH] ARM: OMAP3: gpmc: add BCH ecc api and modes

2012-04-17 Thread Ivan Djelic 
This patch adds a simple BCH ecc computation api, similar to the
existing Hamming ecc api. It is intended to be used by the MTD layer.
It implements the following features:

- support 4-bit and 8-bit ecc computation
- do not protect user bytes in spare area, only data area is protected
- ecc for an erased NAND page (0xFFs) is also a sequence of 0xFFs

This last feature is obtained by adding a constant polynomial to
the hardware computed ecc. It allows to correct bitflips in blank pages
and is extremely useful to support filesystems such as UBIFS, which expect
erased pages to contain only 0xFFs.

This api has been tested on an OMAP3630 board.

Signed-off-by: Ivan Djelic ivan.dje...@parrot.com
---
 arch/arm/mach-omap2/gpmc.c |  148 
 arch/arm/plat-omap/include/plat/gpmc.h |   10 +++
 2 files changed, 158 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 00d5108..dc9a9a4 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -49,6 +49,7 @@
 #define GPMC_ECC_CONTROL   0x1f8
 #define GPMC_ECC_SIZE_CONFIG   0x1fc
 #define GPMC_ECC1_RESULT0x200
+#define GPMC_ECC_BCH_RESULT_0   0x240
 
 #define GPMC_CS0_OFFSET0x60
 #define GPMC_CS_SIZE   0x30
@@ -920,3 +921,150 @@ int gpmc_calculate_ecc(int cs, const u_char *dat, u_char 
*ecc_code)
return 0;
 }
 EXPORT_SYMBOL_GPL(gpmc_calculate_ecc);
+
+#ifdef CONFIG_ARCH_OMAP3
+
+/**
+ * gpmc_enable_hwecc_bch - enable hardware BCH ecc functionality
+ * @cs: chip select number
+ * @mode: read/write mode
+ * @dev_width: device bus width(1 for x16, 0 for x8)
+ * @nsectors: how many 512-byte sectors to process
+ * @nerrors: how many errors to correct per sector (4 or 8)
+ */
+int gpmc_enable_hwecc_bch(int cs, int mode, int dev_width, int nsectors,
+ int nerrors)
+{
+   unsigned int val;
+
+   /* check if ecc module is in use */
+   if (gpmc_ecc_used != -EINVAL)
+   return -EINVAL;
+
+   /*
+* FIXME: some OMAP3 revisions have a hardware bug which prevents
+* the 4-bit BCH mode to work properly. Such revisions should be
+* detected and rejected here.
+*/
+
+   gpmc_ecc_used = cs;
+
+   /* clear ecc and enable bits */
+   gpmc_write_reg(GPMC_ECC_CONTROL, 0x101);
+
+   /*
+* When using BCH, sector size is hardcoded to 512 bytes.
+* Here we are using wrapping mode 6 both for reading and writing, with:
+*  size0 = 0  (no additional protected byte in spare area)
+*  size1 = 32 (skip 32 nibbles = 16 bytes per sector in spare area)
+*/
+   gpmc_write_reg(GPMC_ECC_SIZE_CONFIG, (32  22) | (0  12));
+
+   /* BCH configuration */
+   val = ((1 16) | /* enable BCH */
+  (((nerrors == 8) ? 1 : 0)  12) | /* 8 or 4 bits */
+  (0x06   8) | /* wrap mode = 6 */
+  (dev_width  7) | /* bus width */
+  (((nsectors-1)  0x7)   4) | /* number of sectors */
+  (cs 1) | /* ECC CS */
+  (0x1));/* enable ECC */
+
+   gpmc_write_reg(GPMC_ECC_CONFIG, val);
+   return 0;
+}
+EXPORT_SYMBOL_GPL(gpmc_enable_hwecc_bch);
+
+/**
+ * gpmc_calculate_ecc_bch4 - Generate 7 ecc bytes per sector of 512 data bytes
+ * @cs:  chip select number
+ * @dat: The pointer to data on which ecc is computed
+ * @ecc: The ecc output buffer
+ */
+int gpmc_calculate_ecc_bch4(int cs, const u_char *dat, u_char *ecc)
+{
+   int i;
+   unsigned long nsectors, reg, val1, val2;
+
+   if (gpmc_ecc_used != cs)
+   return -EINVAL;
+
+   nsectors = ((gpmc_read_reg(GPMC_ECC_CONFIG)  4)  0x7) + 1;
+
+   for (i = 0; i  nsectors; i++) {
+
+   reg = GPMC_ECC_BCH_RESULT_0 + 16*i;
+
+   /* Read hw-computed remainder */
+   val1 = gpmc_read_reg(reg + 0);
+   val2 = gpmc_read_reg(reg + 4);
+
+   /*
+* Add constant polynomial to remainder, in order to get an ecc
+* sequence of 0xFFs for a buffer filled with 0xFFs; and
+* left-justify the resulting polynomial.
+*/
+   *ecc++ = 0x28 ^ ((val2  12)  0xFF);
+   *ecc++ = 0x13 ^ ((val2   4)  0xFF);
+   *ecc++ = 0xcc ^ (((val2  0xF)  4)|((val1  28)  0xF));
+   *ecc++ = 0x39 ^ ((val1  20)  0xFF);
+   *ecc++ = 0x96 ^ ((val1  12)  0xFF);
+   *ecc++ = 0xac ^ ((val1  4)  0xFF);
+   *ecc++ = 0x7f ^ ((val1  0xF)  4);
+   }
+
+   gpmc_ecc_used = -EINVAL;
+   return 0;
+}
+EXPORT_SYMBOL_GPL(gpmc_calculate_ecc_bch4);
+
+/**
+ * gpmc_calculate_ecc_bch8 - Generate 13 ecc bytes per block of 512 data bytes
+ * @cs:  chip select number
+ * @dat: The pointer to data on which ecc is computed
+ * @ecc