Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1534655

Re: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc()

From Masahiro Yamada <yamada.masahiro@socionext.com>
Newsgroups linux.kernel
Subject Re: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc()
Date 2016-12-02 05:30 +0100
Message-ID <sJNDX-739-3@gated-at.bofh.it> (permalink)
References <sHPAd-1yq-5@gated-at.bofh.it> <sHPJU-1GL-21@gated-at.bofh.it> <sI9Sh-66y-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hi Boris,


2016-11-28 0:42 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
>> +                     if (err_byte < ECC_SECTOR_SIZE) {
>> +                             struct mtd_info *mtd =
>> +                                     nand_to_mtd(&denali->nand);
>> +                             int offset;
>> +
>> +                             offset = (err_sector * ECC_SECTOR_SIZE + err_byte) *
>> +                                     denali->devnum + err_device;
>> +                             /* correct the ECC error */
>> +                             buf[offset] ^= err_correction_value;
>> +                             mtd->ecc_stats.corrected++;
>> +                             bitflips++;
>
> Hm, bitflips is what is set in max_bitflips, and apparently the
> implementation (which is not yours) is not doing what the core expects.
>
> You should first count bitflips per sector with something like that:
>
>                                 bitflips[err_sector]++;
>
>
> And then once you've iterated over all errors do:
>
>         for (i = 0; i < nsectors; i++)
>                 max_bitflips = max(bitflips[err_sector], max_bitflips);


I see.

For soft ECC fixup, we can calculate bitflips
for each ECC sector, so I can fix the max_bitflips
as the core framework expects.

For hard ECC fixup, the register only reports
the number of corrected bit-flips
in the whole page (sum from all ECC sectors).
We cannot calculate max_bitflips, I think.



BTW, I noticed another problem of the current code.

      buf[offset] ^= err_correction_value;
      mtd->ecc_stats.corrected++;
      bitflips++;

This code is counting the number of corrected bytes,
not the number of corrected bits.


I think multiple bit-flips within one byte can happen.


Perhaps, we should add

  hweight8(buf[offset] ^ err_correction_value)

to ecc_stats.corrected and bitflips.




-- 
Best Regards
Masahiro Yamada

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 09/39] mtd: nand: denali: fix erased page check code Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 09/39] mtd: nand: denali: fix erased page check code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 16:30 +0100
      Re: [PATCH 09/39] mtd: nand: denali: fix erased page check code Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-02 05:40 +0100
        Re: [PATCH 09/39] mtd: nand: denali: fix erased page check code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-12-02 09:00 +0100
  [PATCH 22/39] mtd: nand: denali_dt: remove dma-mask DT property Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 22/39] mtd: nand: denali_dt: remove dma-mask DT property Rob Herring <robh@kernel.org> - 2016-12-01 17:00 +0100
  [PATCH 29/39] mtd: nand: denali: refactor multi NAND fixup code in more generic way Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to  a helper function Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 17:30 +0100
      Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a  helper function Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-30 07:10 +0100
        Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to  a helper function Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-30 09:10 +0100
  [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 15/39] mtd: nand: denali: improve readability of  handle_ecc() Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 16:40 +0100
    Re: [PATCH 15/39] mtd: nand: denali: improve readability of  handle_ecc() Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 16:50 +0100
      Re: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-02 05:30 +0100
        Re: [PATCH 15/39] mtd: nand: denali: improve readability of  handle_ecc() Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-12-02 09:00 +0100
  [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw() above denali_read_page() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw()  above denali_read_page() Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 17:20 +0100
      Re: [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw()  above denali_read_page() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-30 07:20 +0100
  [PATCH 37/39] mtd: nand: denali: support "nand-ecc-strength" DT property Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 37/39] mtd: nand: denali: support "nand-ecc-strength" DT  property Rob Herring <robh@kernel.org> - 2016-12-01 17:20 +0100
  [PATCH 23/39] mtd: nand: denali_dt: use pdev instead of ofdev for platform_device Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 36/39] mtd: nand: denali: allow to use SoC-specific ECC strength Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 02/39] mtd: nand: denali: remove unused CONFIG option and macros Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 30/39] mtd: nand: denali: set DEVICES_CONNECTED 1 if not set Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 27/39] mtd: nand: denali: do not set mtd->name Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 35/39] mtd: nand: denali: calculate ecc.strength and ecc.bytes generically Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Rob Herring <robh@kernel.org> - 2016-12-01 17:10 +0100
      Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-02 04:00 +0100
        Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Rob Herring <robh@kernel.org> - 2016-12-02 17:30 +0100
          Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-03 03:50 +0100
            Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Marek Vasut <marek.vasut@gmail.com> - 2016-12-03 04:00 +0100
              Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Dinh Nguyen <dinh.linux@gmail.com> - 2016-12-03 23:10 +0100
                Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-05 04:40 +0100
                Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Marek Vasut <marek.vasut@gmail.com> - 2016-12-05 04:50 +0100
                Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-05 05:20 +0100
                Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Marek Vasut <marek.vasut@gmail.com> - 2016-12-05 05:30 +0100
                Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Dinh Nguyen <dinh.linux@gmail.com> - 2016-12-05 22:00 +0100
                Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Marek Vasut <marek.vasut@gmail.com> - 2016-12-05 22:50 +0100
                Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Dinh Nguyen <dinguyen@kernel.org> - 2016-12-05 23:40 +0100
  [PATCH 12/39] mtd: nand: denali: return 0 for uncorrectable ECC error Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 31/39] mtd: nand: denali: remove meaningless writes to read-only registers Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 32/39] mtd: nand: denali: remove unnecessary writes to ECC_CORRECTION Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 01/39] mtd: nand: allow to set only one of ECC size and ECC strength from DT Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 01/39] mtd: nand: allow to set only one of ECC size and  ECC strength from DT Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 15:00 +0100
  [PATCH 06/39] mtd: nand: denali: fix write_oob_data() function Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 03/39] mtd: nand: denali: remove redundant define of BANK(x) Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 05/39] mtd: nand: denali: fix comment of denali_nand_info::flash_mem Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 14/39] mtd: nand: denali: replace uint{8/16/32}_t with u{8/16/32} Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 24/39] mtd: nand: denali: add NEW_N_BANKS_FORMAT capability Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 26/39] mtd: nand: denali: call nand_set_flash_node() to set DT node Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 33/39] mtd: nand: denali: support 1024 byte ECC step size Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 33/39] mtd: nand: denali: support 1024 byte ECC step size Rob Herring <robh@kernel.org> - 2016-12-01 17:00 +0100
  [PATCH 10/39] mtd: nand: denali: remove redundant if conditional of erased_check Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
  [PATCH 38/39] mtd: nand: denali: remove Toshiba, Hynix specific fixup code Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    Re: [PATCH 38/39] mtd: nand: denali: remove Toshiba, Hynix specific  fixup code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 17:30 +0100
  Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 16:10 +0100
    Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-30 09:10 +0100
      Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-30 09:20 +0100
        Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-01 10:20 +0100
    Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-30 09:20 +0100
  Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 17:40 +0100

csiph-web