Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1530888
| From | Boris Brezillon <boris.brezillon@free-electrons.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc() |
| Date | 2016-11-27 16:50 +0100 |
| Message-ID | <sI9Sh-66y-1@gated-at.bofh.it> (permalink) |
| References | <sHPAd-1yq-5@gated-at.bofh.it> <sHPJU-1GL-21@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Sun, 27 Nov 2016 03:06:01 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> This function is unreadable due to the deep nesting. Note this
> function does a job only when INTR_STATUS__ECC_ERR is set.
> So, if the flag is not set, let it bail-out.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> drivers/mtd/nand/denali.c | 119 +++++++++++++++++++++++-----------------------
> 1 file changed, 59 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index a7dc692..b577560 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -908,69 +908,68 @@ static bool handle_ecc(struct denali_nand_info *denali, u8 *buf,
> {
> bool check_erased_page = false;
> unsigned int bitflips = 0;
> + u32 err_address, err_correction_info, err_byte, err_sector, err_device,
> + err_correction_value;
>
> - if (irq_status & INTR_STATUS__ECC_ERR) {
> - /* read the ECC errors. we'll ignore them for now */
> - u32 err_address, err_correction_info, err_byte,
> - err_sector, err_device, err_correction_value;
> - denali_set_intr_modes(denali, false);
> -
> - do {
> - err_address = ioread32(denali->flash_reg +
> - ECC_ERROR_ADDRESS);
> - err_sector = ECC_SECTOR(err_address);
> - err_byte = ECC_BYTE(err_address);
> -
> - err_correction_info = ioread32(denali->flash_reg +
> - ERR_CORRECTION_INFO);
> - err_correction_value =
> + if (!(irq_status & INTR_STATUS__ECC_ERR))
> + goto out;
> +
> + /* read the ECC errors. we'll ignore them for now */
> + denali_set_intr_modes(denali, false);
> +
> + do {
> + err_address = ioread32(denali->flash_reg + ECC_ERROR_ADDRESS);
> + err_sector = ECC_SECTOR(err_address);
> + err_byte = ECC_BYTE(err_address);
> +
> + err_correction_info = ioread32(denali->flash_reg +
> + ERR_CORRECTION_INFO);
> + err_correction_value =
> ECC_CORRECTION_VALUE(err_correction_info);
> - err_device = ECC_ERR_DEVICE(err_correction_info);
> -
> - if (ECC_ERROR_CORRECTABLE(err_correction_info)) {
> - /*
> - * If err_byte is larger than ECC_SECTOR_SIZE,
> - * means error happened in OOB, so we ignore
> - * it. It's no need for us to correct it
> - * err_device is represented the NAND error
> - * bits are happened in if there are more
> - * than one NAND connected.
> - */
> - 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++;
> - }
> - } else {
> - /*
> - * if the error is not correctable, need to
> - * look at the page to see if it is an erased
> - * page. if so, then it's not a real ECC error
> - */
> - check_erased_page = true;
> + err_device = ECC_ERR_DEVICE(err_correction_info);
> +
> + if (ECC_ERROR_CORRECTABLE(err_correction_info)) {
> + /*
> + * If err_byte is larger than ECC_SECTOR_SIZE, means error
> + * happened in OOB, so we ignore it. It's no need for
> + * us to correct it err_device is represented the NAND
> + * error bits are happened in if there are more than
> + * one NAND connected.
> + */
> + 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);
> }
> - } while (!ECC_LAST_ERR(err_correction_info));
> - /*
> - * Once handle all ecc errors, controller will triger
> - * a ECC_TRANSACTION_DONE interrupt, so here just wait
> - * for a while for this interrupt
> - */
> - while (!(read_interrupt_status(denali) &
> - INTR_STATUS__ECC_TRANSACTION_DONE))
> - cpu_relax();
> - clear_interrupts(denali);
> - denali_set_intr_modes(denali, true);
> - }
> + } else {
> + /*
> + * if the error is not correctable, need to look at the
> + * page to see if it is an erased page. if so, then
> + * it's not a real ECC error
> + */
> + check_erased_page = true;
> + }
> + } while (!ECC_LAST_ERR(err_correction_info));
> +
> + /*
> + * Once handle all ecc errors, controller will triger a
> + * ECC_TRANSACTION_DONE interrupt, so here just wait for
> + * a while for this interrupt
> + */
> + while (!(read_interrupt_status(denali) &
> + INTR_STATUS__ECC_TRANSACTION_DONE))
> + cpu_relax();
> + clear_interrupts(denali);
> + denali_set_intr_modes(denali, true);
> +
> + out:
> *max_bitflips = bitflips;
> return check_erased_page;
> }
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll 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