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


Groups > linux.kernel > #1530723 > unrolled thread

[PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb

Started byMasahiro Yamada <yamada.masahiro@socionext.com>
First post2016-11-26 19:20 +0100
Last post2016-11-27 17:40 +0100
Articles 20 on this page of 64 — 6 participants

Back to article view | Back to linux.kernel


Contents

  [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

Page 1 of 4  [1] 2 3 4  Next page →


#1530723 — [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-11-26 19:20 +0100
Subject[PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb
Message-ID<sHPAd-1yq-5@gated-at.bofh.it>
As I said in the 1st round series, I am tackling on this driver
to use it for my SoCs.

The previous series was just cosmetic things, but this series
includes *real* changes.

After some more cleanups, I will start to add changes that
are really necessary.
One of the biggest problems I want to solve is a bunch of
hard-coded parameters that prevent me from using this driver for
my SoCs.

I will introduce capability flags that are associated with DT
compatible and make platform-dependent parameters overridable.

I still have lots of reworks to get done (so probably 3rd round
series will come), but I hope it is getting better and
I am showing a big picture now.



Masahiro Yamada (39):
  mtd: nand: allow to set only one of ECC size and ECC strength from DT
  mtd: nand: denali: remove unused CONFIG option and macros
  mtd: nand: denali: remove redundant define of BANK(x)
  mtd: nand: denali: remove more unused struct members
  mtd: nand: denali: fix comment of denali_nand_info::flash_mem
  mtd: nand: denali: fix write_oob_data() function
  mtd: nand: denali: transfer OOB only when oob_required is set
  mtd: nand: denali: introduce capability flag
  mtd: nand: denali: fix erased page check code
  mtd: nand: denali: remove redundant if conditional of erased_check
  mtd: nand: denali: increment ecc_stats.failed by one per error
  mtd: nand: denali: return 0 for uncorrectable ECC error
  mtd: nand: denali: increment ecc_stats->corrected
  mtd: nand: denali: replace uint{8/16/32}_t with u{8/16/32}
  mtd: nand: denali: improve readability of handle_ecc()
  mtd: nand: denali: rename handle_ecc() to denali_sw_ecc_fixup()
  mtd: nand: denali: support HW_ECC_FIXUP capability
  mtd: nand: denali: move denali_read_page_raw() above
    denali_read_page()
  mtd: nand: denali: perform erased check against raw transferred page
  mtd: nand: denali_dt: enable HW_ECC_FIXUP capability for DT platform
  mtd: nand: denali: support 64bit capable DMA engine
  mtd: nand: denali_dt: remove dma-mask DT property
  mtd: nand: denali_dt: use pdev instead of ofdev for platform_device
  mtd: nand: denali: add NEW_N_BANKS_FORMAT capability
  mtd: nand: denali: use nand_chip to hold frequently accessed data
  mtd: nand: denali: call nand_set_flash_node() to set DT node
  mtd: nand: denali: do not set mtd->name
  mtd: nand: denali: move multi NAND fixup code to a helper function
  mtd: nand: denali: refactor multi NAND fixup code in more generic way
  mtd: nand: denali: set DEVICES_CONNECTED 1 if not set
  mtd: nand: denali: remove meaningless writes to read-only registers
  mtd: nand: denali: remove unnecessary writes to ECC_CORRECTION
  mtd: nand: denali: support 1024 byte ECC step size
  mtd: nand: denali: fix the condition for 15 bit ECC strength
  mtd: nand: denali: calculate ecc.strength and ecc.bytes generically
  mtd: nand: denali: allow to use SoC-specific ECC strength
  mtd: nand: denali: support "nand-ecc-strength" DT property
  mtd: nand: denali: remove Toshiba, Hynix specific fixup code
  mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

 .../devicetree/bindings/mtd/denali-nand.txt        |  19 +-
 drivers/mtd/nand/Kconfig                           |  11 -
 drivers/mtd/nand/denali.c                          | 740 ++++++++++++---------
 drivers/mtd/nand/denali.h                          |  84 +--
 drivers/mtd/nand/denali_dt.c                       |  95 ++-
 drivers/mtd/nand/denali_pci.c                      |   2 +
 drivers/mtd/nand/nand_base.c                       |   6 -
 7 files changed, 515 insertions(+), 442 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]


#1530724 — [PATCH 09/39] mtd: nand: denali: fix erased page check code

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-11-26 19:20 +0100
Subject[PATCH 09/39] mtd: nand: denali: fix erased page check code
Message-ID<sHPJT-1GL-11@gated-at.bofh.it>
In reply to#1530723
Currently, is_erased() is called against "buf" twice, so the second
call is meaningless.  The second one should be checked against
chip->oob_poi.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/denali.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index cbc7f75..753e9a02 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1160,7 +1160,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 		if (check_erased_page) {
 			if (!is_erased(buf, mtd->writesize))
 				mtd->ecc_stats.failed++;
-			if (!is_erased(buf, mtd->oobsize))
+			if (!is_erased(chip->oob_poi, mtd->oobsize))
 				mtd->ecc_stats.failed++;
 		}
 	}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1530885 — Re: [PATCH 09/39] mtd: nand: denali: fix erased page check code

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-11-27 16:30 +0100
SubjectRe: [PATCH 09/39] mtd: nand: denali: fix erased page check code
Message-ID<sI9yV-606-11@gated-at.bofh.it>
In reply to#1530724
On Sun, 27 Nov 2016 03:05:55 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Currently, is_erased() is called against "buf" twice, so the second
> call is meaningless.  The second one should be checked against
> chip->oob_poi.
> 

IMO, patch 9 to 12 should be squashed in a single patch. All you're
doing in these patch is fixing the check_erased_page logic.

You can describe the different broken thing in the commit message, but
splitting things as you do does not help much.

Also, please have at nand_check_erased_ecc_chunk() [1] instead of using
a private method (is_erased()) to check if the page is erased.
With this method you get bitflips in erased pages correction for free.

[1]http://lxr.free-electrons.com/source/drivers/mtd/nand/nand_base.c#L1212

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/mtd/nand/denali.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index cbc7f75..753e9a02 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1160,7 +1160,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  		if (check_erased_page) {
>  			if (!is_erased(buf, mtd->writesize))
>  				mtd->ecc_stats.failed++;
> -			if (!is_erased(buf, mtd->oobsize))
> +			if (!is_erased(chip->oob_poi, mtd->oobsize))
>  				mtd->ecc_stats.failed++;
>  		}
>  	}

[toc] | [prev] | [next] | [standalone]


#1534662 — Re: [PATCH 09/39] mtd: nand: denali: fix erased page check code

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-12-02 05:40 +0100
SubjectRe: [PATCH 09/39] mtd: nand: denali: fix erased page check code
Message-ID<sJNND-76a-5@gated-at.bofh.it>
In reply to#1530885
Hi Boris,


2016-11-28 0:21 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> On Sun, 27 Nov 2016 03:05:55 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
>> Currently, is_erased() is called against "buf" twice, so the second
>> call is meaningless.  The second one should be checked against
>> chip->oob_poi.
>>
>
> IMO, patch 9 to 12 should be squashed in a single patch. All you're
> doing in these patch is fixing the check_erased_page logic.
>
> You can describe the different broken thing in the commit message, but
> splitting things as you do does not help much.


OK. I will do so.

I realized some mistakes in this part
(both in my patches and in the current mainline code),
so I will rework it in a more sensible chunk.


> Also, please have at nand_check_erased_ecc_chunk() [1] instead of using
> a private method (is_erased()) to check if the page is erased.
> With this method you get bitflips in erased pages correction for free.

I will use this helper, thanks!




With this, I think I answered all of your questions to v1.

(Please tell me if there is something I missed to answer.)

Thanks a lot for your review.



-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]


#1534719 — Re: [PATCH 09/39] mtd: nand: denali: fix erased page check code

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-12-02 09:00 +0100
SubjectRe: [PATCH 09/39] mtd: nand: denali: fix erased page check code
Message-ID<sJQVg-HL-3@gated-at.bofh.it>
In reply to#1534662
On Fri, 2 Dec 2016 13:33:58 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi Boris,
> 
> 
> 2016-11-28 0:21 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> > On Sun, 27 Nov 2016 03:05:55 +0900
> > Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> >  
> >> Currently, is_erased() is called against "buf" twice, so the second
> >> call is meaningless.  The second one should be checked against
> >> chip->oob_poi.
> >>  
> >
> > IMO, patch 9 to 12 should be squashed in a single patch. All you're
> > doing in these patch is fixing the check_erased_page logic.
> >
> > You can describe the different broken thing in the commit message, but
> > splitting things as you do does not help much.  
> 
> 
> OK. I will do so.
> 
> I realized some mistakes in this part
> (both in my patches and in the current mainline code),
> so I will rework it in a more sensible chunk.
> 
> 
> > Also, please have at nand_check_erased_ecc_chunk() [1] instead of using
> > a private method (is_erased()) to check if the page is erased.
> > With this method you get bitflips in erased pages correction for free.  
> 
> I will use this helper, thanks!
> 
> 
> 
> 
> With this, I think I answered all of your questions to v1.

You did. I'm waiting for the v2 now ;)

> 
> (Please tell me if there is something I missed to answer.)
> 
> Thanks a lot for your review.
> 
> 
> 

[toc] | [prev] | [next] | [standalone]


#1530725 — [PATCH 22/39] mtd: nand: denali_dt: remove dma-mask DT property

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-11-26 19:20 +0100
Subject[PATCH 22/39] mtd: nand: denali_dt: remove dma-mask DT property
Message-ID<sHPJU-1GL-15@gated-at.bofh.it>
In reply to#1530723
The driver sets appropriate DMA mask.  Delete the "dma-mask" DT
property.  Refer to the Link tag for negative opinions for this
binding.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Link: https://lkml.org/lkml/2016/2/8/57
---

 Documentation/devicetree/bindings/mtd/denali-nand.txt | 2 --
 drivers/mtd/nand/denali_dt.c                          | 9 ---------
 2 files changed, 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index b04d03a..603110b 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -5,7 +5,6 @@ Required properties:
   - reg : should contain registers location and length for data and reg.
   - reg-names: Should contain the reg names "nand_data" and "denali_reg"
   - interrupts : The interrupt number.
-  - dm-mask : DMA bit mask
 
 The device tree may optionally contain sub-nodes describing partitions of the
 address space. See partition.txt for more detail.
@@ -19,5 +18,4 @@ nand: nand@ff900000 {
 	reg = <0xff900000 0x100000>, <0xffb80000 0x10000>;
 	reg-names = "nand_data", "denali_reg";
 	interrupts = <0 144 4>;
-	dma-mask = <0xffffffff>;
 };
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index 9dcd203..6a486a7 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -40,8 +40,6 @@ static const struct of_device_id denali_nand_dt_ids[] = {
 
 MODULE_DEVICE_TABLE(of, denali_nand_dt_ids);
 
-static u64 denali_dma_mask;
-
 static int denali_dt_probe(struct platform_device *ofdev)
 {
 	struct resource *denali_reg, *nand_data;
@@ -79,13 +77,6 @@ static int denali_dt_probe(struct platform_device *ofdev)
 	if (IS_ERR(denali->flash_mem))
 		return PTR_ERR(denali->flash_mem);
 
-	if (!of_property_read_u32(ofdev->dev.of_node,
-		"dma-mask", (u32 *)&denali_dma_mask)) {
-		denali->dev->dma_mask = &denali_dma_mask;
-	} else {
-		denali->dev->dma_mask = NULL;
-	}
-
 	dt->clk = devm_clk_get(&ofdev->dev, NULL);
 	if (IS_ERR(dt->clk)) {
 		dev_err(&ofdev->dev, "no clk available\n");
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1534233 — Re: [PATCH 22/39] mtd: nand: denali_dt: remove dma-mask DT property

FromRob Herring <robh@kernel.org>
Date2016-12-01 17:00 +0100
SubjectRe: [PATCH 22/39] mtd: nand: denali_dt: remove dma-mask DT property
Message-ID<sJBWa-5Xk-27@gated-at.bofh.it>
In reply to#1530725
On Sun, Nov 27, 2016 at 03:06:08AM +0900, Masahiro Yamada wrote:
> The driver sets appropriate DMA mask.  Delete the "dma-mask" DT
> property.  Refer to the Link tag for negative opinions for this
> binding.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Link: https://lkml.org/lkml/2016/2/8/57
> ---
> 
>  Documentation/devicetree/bindings/mtd/denali-nand.txt | 2 --
>  drivers/mtd/nand/denali_dt.c                          | 9 ---------
>  2 files changed, 11 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

[toc] | [prev] | [next] | [standalone]


#1530726 — [PATCH 29/39] mtd: nand: denali: refactor multi NAND fixup code in more generic way

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-11-26 19:20 +0100
Subject[PATCH 29/39] mtd: nand: denali: refactor multi NAND fixup code in more generic way
Message-ID<sHPJT-1GL-13@gated-at.bofh.it>
In reply to#1530723
Currently, this code expects denali->devnum == 1 or 2, but it can be
written in a more generic way, although denali->devnum should be a
power of 2.

Also, make the code more readable with devnum_shift.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/denali.c | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 54dcd83..71bdf2a 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1476,6 +1476,7 @@ static void denali_multidev_fixup(struct denali_nand_info *denali)
 {
 	struct nand_chip *chip = &denali->nand;
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	int devnum_shift;
 
 	/*
 	 * Support for multi NAND:
@@ -1484,20 +1485,30 @@ static void denali_multidev_fixup(struct denali_nand_info *denali)
 	 */
 	denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
 
-	mtd->size <<= denali->devnum - 1;
-	mtd->erasesize <<= denali->devnum - 1;
-	mtd->writesize <<= denali->devnum - 1;
-	mtd->oobsize <<= denali->devnum - 1;
-	chip->chipsize <<= denali->devnum - 1;
-	chip->page_shift += denali->devnum - 1;
-	chip->phys_erase_shift += denali->devnum - 1;
-	chip->bbt_erase_shift += denali->devnum - 1;
-	chip->chip_shift += denali->devnum - 1;
-	chip->pagemask <<= denali->devnum - 1;
-	chip->ecc.size *= denali->devnum;
-	chip->ecc.bytes *= denali->devnum;
-	chip->ecc.strength *= denali->devnum;
-	denali->bbtskipbytes *= denali->devnum;
+	devnum_shift = fls(denali->devnum) - 1;
+
+	if (denali->devnum != BIT(devnum_shift)) {
+		dev_warn(denali->dev,
+			 "DEVICE_CONNECTED=%d is not power of 2. Some devices will not be used.\n",
+			 denali->devnum);
+		denali->devnum = BIT(devnum_shift);
+		iowrite32(denali->devnum, denali->flash_reg + DEVICES_CONNECTED);
+	}
+
+	mtd->size <<= devnum_shift;
+	mtd->erasesize <<= devnum_shift;
+	mtd->writesize <<= devnum_shift;
+	mtd->oobsize <<= devnum_shift;
+	chip->chipsize <<= devnum_shift;
+	chip->page_shift += devnum_shift;
+	chip->phys_erase_shift += devnum_shift;
+	chip->bbt_erase_shift += devnum_shift;
+	chip->chip_shift += devnum_shift;
+	chip->pagemask <<= devnum_shift;
+	chip->ecc.size <<= devnum_shift;
+	chip->ecc.bytes <<= devnum_shift;
+	chip->ecc.strength <<= devnum_shift;
+	denali->bbtskipbytes <<= devnum_shift;
 }
 
 int denali_init(struct denali_nand_info *denali)
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1530727 — [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-11-26 19:20 +0100
Subject[PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function
Message-ID<sHPJU-1GL-17@gated-at.bofh.it>
In reply to#1530723
Collect multi NAND fixups into a helper function instead of
scattering them in denali_init().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/denali.c | 51 ++++++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 60b0858..54dcd83 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1472,6 +1472,34 @@ static void denali_drv_init(struct denali_nand_info *denali)
 	denali->irq_status = 0;
 }
 
+static void denali_multidev_fixup(struct denali_nand_info *denali)
+{
+	struct nand_chip *chip = &denali->nand;
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
+	/*
+	 * Support for multi NAND:
+	 * MTD knows nothing about multi NAND, so we should tell it
+	 * the real pagesize and anything necessary
+	 */
+	denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
+
+	mtd->size <<= denali->devnum - 1;
+	mtd->erasesize <<= denali->devnum - 1;
+	mtd->writesize <<= denali->devnum - 1;
+	mtd->oobsize <<= denali->devnum - 1;
+	chip->chipsize <<= denali->devnum - 1;
+	chip->page_shift += denali->devnum - 1;
+	chip->phys_erase_shift += denali->devnum - 1;
+	chip->bbt_erase_shift += denali->devnum - 1;
+	chip->chip_shift += denali->devnum - 1;
+	chip->pagemask <<= denali->devnum - 1;
+	chip->ecc.size *= denali->devnum;
+	chip->ecc.bytes *= denali->devnum;
+	chip->ecc.strength *= denali->devnum;
+	denali->bbtskipbytes *= denali->devnum;
+}
+
 int denali_init(struct denali_nand_info *denali)
 {
 	struct nand_chip *chip = &denali->nand;
@@ -1553,23 +1581,6 @@ int denali_init(struct denali_nand_info *denali)
 		goto failed_req_irq;
 	}
 
-	/*
-	 * support for multi nand
-	 * MTD known nothing about multi nand, so we should tell it
-	 * the real pagesize and anything necessery
-	 */
-	denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
-	chip->chipsize <<= denali->devnum - 1;
-	chip->page_shift += denali->devnum - 1;
-	chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
-	chip->bbt_erase_shift += denali->devnum - 1;
-	chip->phys_erase_shift = chip->bbt_erase_shift;
-	chip->chip_shift += denali->devnum - 1;
-	mtd->writesize <<= denali->devnum - 1;
-	mtd->oobsize <<= denali->devnum - 1;
-	mtd->erasesize <<= denali->devnum - 1;
-	mtd->size = chip->numchips * chip->chipsize;
-	denali->bbtskipbytes *= denali->devnum;
 
 	/*
 	 * second stage of the NAND scan
@@ -1614,11 +1625,9 @@ int denali_init(struct denali_nand_info *denali)
 	}
 
 	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
-	chip->ecc.bytes *= denali->devnum;
-	chip->ecc.strength *= denali->devnum;
 
 	/* override the default read operations */
-	chip->ecc.size = ECC_SECTOR_SIZE * denali->devnum;
+	chip->ecc.size = ECC_SECTOR_SIZE;
 	chip->ecc.read_page = denali_read_page;
 	chip->ecc.read_page_raw = denali_read_page_raw;
 	chip->ecc.write_page = denali_write_page;
@@ -1627,6 +1636,8 @@ int denali_init(struct denali_nand_info *denali)
 	chip->ecc.write_oob = denali_write_oob;
 	chip->erase = denali_erase;
 
+	denali_multidev_fixup(denali);
+
 	ret = nand_scan_tail(mtd);
 	if (ret)
 		goto failed_req_irq;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1530900 — Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-11-27 17:30 +0100
SubjectRe: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function
Message-ID<sIauZ-6Ei-5@gated-at.bofh.it>
In reply to#1530727
On Sun, 27 Nov 2016 03:06:14 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Collect multi NAND fixups into a helper function instead of
> scattering them in denali_init().

Can you tell me more about this multi-NAND feature?
The core is already able to detect multi-die NAND chips in a generic
way, but I fear this is something else, like "put two 8-bits chips on a
16bits bus to emulate a single 16bits chip".
If that's a case, and this feature is actually used, then it's a bad
idea IMHO.
For example, how do you handle the case where one block is bad on a
chip but not on the other? And I fear this is not the only problem
with this approach :-/. 

> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/mtd/nand/denali.c | 51 ++++++++++++++++++++++++++++-------------------
>  1 file changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 60b0858..54dcd83 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1472,6 +1472,34 @@ static void denali_drv_init(struct denali_nand_info *denali)
>  	denali->irq_status = 0;
>  }
>  
> +static void denali_multidev_fixup(struct denali_nand_info *denali)
> +{
> +	struct nand_chip *chip = &denali->nand;
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +
> +	/*
> +	 * Support for multi NAND:
> +	 * MTD knows nothing about multi NAND, so we should tell it
> +	 * the real pagesize and anything necessary
> +	 */
> +	denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
> +
> +	mtd->size <<= denali->devnum - 1;
> +	mtd->erasesize <<= denali->devnum - 1;
> +	mtd->writesize <<= denali->devnum - 1;
> +	mtd->oobsize <<= denali->devnum - 1;
> +	chip->chipsize <<= denali->devnum - 1;
> +	chip->page_shift += denali->devnum - 1;
> +	chip->phys_erase_shift += denali->devnum - 1;
> +	chip->bbt_erase_shift += denali->devnum - 1;
> +	chip->chip_shift += denali->devnum - 1;
> +	chip->pagemask <<= denali->devnum - 1;
> +	chip->ecc.size *= denali->devnum;
> +	chip->ecc.bytes *= denali->devnum;
> +	chip->ecc.strength *= denali->devnum;
> +	denali->bbtskipbytes *= denali->devnum;
> +}
> +
>  int denali_init(struct denali_nand_info *denali)
>  {
>  	struct nand_chip *chip = &denali->nand;
> @@ -1553,23 +1581,6 @@ int denali_init(struct denali_nand_info *denali)
>  		goto failed_req_irq;
>  	}
>  
> -	/*
> -	 * support for multi nand
> -	 * MTD known nothing about multi nand, so we should tell it
> -	 * the real pagesize and anything necessery
> -	 */
> -	denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
> -	chip->chipsize <<= denali->devnum - 1;
> -	chip->page_shift += denali->devnum - 1;
> -	chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
> -	chip->bbt_erase_shift += denali->devnum - 1;
> -	chip->phys_erase_shift = chip->bbt_erase_shift;
> -	chip->chip_shift += denali->devnum - 1;
> -	mtd->writesize <<= denali->devnum - 1;
> -	mtd->oobsize <<= denali->devnum - 1;
> -	mtd->erasesize <<= denali->devnum - 1;
> -	mtd->size = chip->numchips * chip->chipsize;
> -	denali->bbtskipbytes *= denali->devnum;
>  
>  	/*
>  	 * second stage of the NAND scan
> @@ -1614,11 +1625,9 @@ int denali_init(struct denali_nand_info *denali)
>  	}
>  
>  	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
> -	chip->ecc.bytes *= denali->devnum;
> -	chip->ecc.strength *= denali->devnum;
>  
>  	/* override the default read operations */
> -	chip->ecc.size = ECC_SECTOR_SIZE * denali->devnum;
> +	chip->ecc.size = ECC_SECTOR_SIZE;
>  	chip->ecc.read_page = denali_read_page;
>  	chip->ecc.read_page_raw = denali_read_page_raw;
>  	chip->ecc.write_page = denali_write_page;
> @@ -1627,6 +1636,8 @@ int denali_init(struct denali_nand_info *denali)
>  	chip->ecc.write_oob = denali_write_oob;
>  	chip->erase = denali_erase;
>  
> +	denali_multidev_fixup(denali);
> +
>  	ret = nand_scan_tail(mtd);
>  	if (ret)
>  		goto failed_req_irq;

[toc] | [prev] | [next] | [standalone]


#1532989 — Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-11-30 07:10 +0100
SubjectRe: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function
Message-ID<sJ6fE-2kZ-17@gated-at.bofh.it>
In reply to#1530900
Hi Boris,


2016-11-28 1:24 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> On Sun, 27 Nov 2016 03:06:14 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
>> Collect multi NAND fixups into a helper function instead of
>> scattering them in denali_init().
>
> Can you tell me more about this multi-NAND feature?
> The core is already able to detect multi-die NAND chips in a generic
> way,

This is not the case.

> but I fear this is something else, like "put two 8-bits chips on a
> 16bits bus to emulate a single 16bits chip".

Yes, it is.

(I have never used this controller like that.
But, I am pretty sure it is
from the code and the
Denali's User Guide mentions such usage.)


Just in case, I will clearly rephrase the comment block like follows in v2:

        /*
         * Support for multi device:
         * When the IP configuration is x16 capable and two x8 chips are
         * connected in parallel, DEVICES_CONNECTED should be set to 2.
         * In this case, the core framework knows nothing about this fact,
         * so we should tell it the _logical_ pagesize and anything necessary.
         */




> If that's a case, and this feature is actually used, then it's a bad
> idea IMHO.
> For example, how do you handle the case where one block is bad on a
> chip but not on the other? And I fear this is not the only problem
> with this approach :-/.

As you expect, if one block is bad,
the correspond block on the other chip can not be used.



-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]


#1533053 — Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-11-30 09:10 +0100
SubjectRe: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function
Message-ID<sJ87M-3xH-21@gated-at.bofh.it>
In reply to#1532989
On Wed, 30 Nov 2016 15:09:27 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi Boris,
> 
> 
> 2016-11-28 1:24 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> > On Sun, 27 Nov 2016 03:06:14 +0900
> > Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> >  
> >> Collect multi NAND fixups into a helper function instead of
> >> scattering them in denali_init().  
> >
> > Can you tell me more about this multi-NAND feature?
> > The core is already able to detect multi-die NAND chips in a generic
> > way,  
> 
> This is not the case.
> 
> > but I fear this is something else, like "put two 8-bits chips on a
> > 16bits bus to emulate a single 16bits chip".  
> 
> Yes, it is.
> 
> (I have never used this controller like that.
> But, I am pretty sure it is
> from the code and the
> Denali's User Guide mentions such usage.)
> 
> 
> Just in case, I will clearly rephrase the comment block like follows in v2:
> 
>         /*
>          * Support for multi device:
>          * When the IP configuration is x16 capable and two x8 chips are
>          * connected in parallel, DEVICES_CONNECTED should be set to 2.
>          * In this case, the core framework knows nothing about this fact,
>          * so we should tell it the _logical_ pagesize and anything necessary.
>          */
> 

BTW, you should also set the NAND_BUSWIDTH_16 flag in this case.

> 
> 
> 
> > If that's a case, and this feature is actually used, then it's a bad
> > idea IMHO.
> > For example, how do you handle the case where one block is bad on a
> > chip but not on the other? And I fear this is not the only problem
> > with this approach :-/.  
> 
> As you expect, if one block is bad,
> the correspond block on the other chip can not be used.
> 

Hm, last time I thought about this usage I found others things that
could cause problems, but I can't remember exactly what.

Anyway, if this feature is already used, let's keep it.

[toc] | [prev] | [next] | [standalone]


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

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-11-26 19:20 +0100
Subject[PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc()
Message-ID<sHPJU-1GL-21@gated-at.bofh.it>
In reply to#1530723
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++;
 			}
-		} 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;
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


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

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-11-27 16:40 +0100
SubjectRe: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc()
Message-ID<sI9IB-63h-9@gated-at.bofh.it>
In reply to#1530728
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;

Please use single line definitions for local variables:

	u32 err_address, err_correction_info, err_byte, err_sector;
	u32 err_device, err_correction_value;

Also, maybe you should use shorter names to avoid 80 chars wrapping as
much as possible.

>  
> -	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++;
>  			}
> -		} 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;
>  }

[toc] | [prev] | [next] | [standalone]


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

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-11-27 16:50 +0100
SubjectRe: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc()
Message-ID<sI9Sh-66y-1@gated-at.bofh.it>
In reply to#1530728
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;
>  }

[toc] | [prev] | [next] | [standalone]


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

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-12-02 05:30 +0100
SubjectRe: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc()
Message-ID<sJNDX-739-3@gated-at.bofh.it>
In reply to#1530888
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

[toc] | [prev] | [next] | [standalone]


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

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-12-02 09:00 +0100
SubjectRe: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc()
Message-ID<sJQVh-HL-19@gated-at.bofh.it>
In reply to#1534655
On Fri, 2 Dec 2016 13:26:27 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> 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.
> 

That's unfortunate. This means you'll return -EUCLEAN more quickly
(which will trigger UBI eraseblock move), since the NAND framework is
basing its 'too many bitflips' detection logic on the max_bitflips per
ECC chunk and the bitflips threshold (by default 3/4 of the ECC
strength).

That doesn't mean it won't work, you'll just wear your NAND more
quickly :-(.

ITOH, doing max_bitflips = nbitflips / nsteps is not good either,
because the bitflips might be all concentrated in the same ECC chunk,
and in this case you really want to return -EUCLEAN.

> 
> 
> 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.

Yes.

> 
> 
> Perhaps, we should add
> 
>   hweight8(buf[offset] ^ err_correction_value)
> 
> to ecc_stats.corrected and bitflips.
> 

Looks good.

[toc] | [prev] | [next] | [standalone]


#1530730 — [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw() above denali_read_page()

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-11-26 19:20 +0100
Subject[PATCH 18/39] mtd: nand: denali: move denali_read_page_raw() above denali_read_page()
Message-ID<sHPJU-1GL-23@gated-at.bofh.it>
In reply to#1530723
This will be needed in the next commit to call denali_read_page_raw()
from denali_read_page().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/denali.c | 76 +++++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index c101e7f..f035dac 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1118,6 +1118,44 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 	return 0;
 }
 
+static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				u8 *buf, int oob_required, int page)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	dma_addr_t addr = denali->buf.dma_buf;
+	size_t size = mtd->writesize + mtd->oobsize;
+	u32 irq_mask = INTR_STATUS__DMA_CMD_COMP;
+
+	if (page != denali->page) {
+		dev_err(denali->dev,
+			"IN %s: page %d is not equal to denali->page %d",
+			__func__, page, denali->page);
+		BUG();
+	}
+
+	setup_ecc_for_xfer(denali, false, oob_required ? true : false);
+	denali_enable_dma(denali, true);
+
+	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
+
+	clear_interrupts(denali);
+	denali_setup_dma(denali, DENALI_READ);
+
+	/* wait for operation to complete */
+	wait_for_irq(denali, irq_mask);
+
+	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
+
+	denali_enable_dma(denali, false);
+
+	memcpy(buf, denali->buf.buf, mtd->writesize);
+	if (oob_required)
+		memcpy(chip->oob_poi, denali->buf.buf + mtd->writesize,
+		       mtd->oobsize);
+
+	return 0;
+}
+
 static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			    u8 *buf, int oob_required, int page)
 {
@@ -1182,44 +1220,6 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	return max_bitflips;
 }
 
-static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-				u8 *buf, int oob_required, int page)
-{
-	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dma_addr_t addr = denali->buf.dma_buf;
-	size_t size = mtd->writesize + mtd->oobsize;
-	u32 irq_mask = INTR_STATUS__DMA_CMD_COMP;
-
-	if (page != denali->page) {
-		dev_err(denali->dev,
-			"IN %s: page %d is not equal to denali->page %d",
-			__func__, page, denali->page);
-		BUG();
-	}
-
-	setup_ecc_for_xfer(denali, false, oob_required ? true : false);
-	denali_enable_dma(denali, true);
-
-	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
-
-	clear_interrupts(denali);
-	denali_setup_dma(denali, DENALI_READ);
-
-	/* wait for operation to complete */
-	wait_for_irq(denali, irq_mask);
-
-	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
-
-	denali_enable_dma(denali, false);
-
-	memcpy(buf, denali->buf.buf, mtd->writesize);
-	if (oob_required)
-		memcpy(chip->oob_poi, denali->buf.buf + mtd->writesize,
-		       mtd->oobsize);
-
-	return 0;
-}
-
 static u8 denali_read_byte(struct mtd_info *mtd)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1530895 — Re: [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw() above denali_read_page()

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-11-27 17:20 +0100
SubjectRe: [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw() above denali_read_page()
Message-ID<sIalk-6B5-5@gated-at.bofh.it>
In reply to#1530730
On Sun, 27 Nov 2016 03:06:04 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> This will be needed in the next commit to call denali_read_page_raw()
> from denali_read_page().

Please squash this change into patch 19. It's clearly useless to
dissociate them.

> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/mtd/nand/denali.c | 76 +++++++++++++++++++++++------------------------
>  1 file changed, 38 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index c101e7f..f035dac 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1118,6 +1118,44 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  	return 0;
>  }
>  
> +static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> +				u8 *buf, int oob_required, int page)
> +{
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
> +	dma_addr_t addr = denali->buf.dma_buf;
> +	size_t size = mtd->writesize + mtd->oobsize;
> +	u32 irq_mask = INTR_STATUS__DMA_CMD_COMP;
> +
> +	if (page != denali->page) {
> +		dev_err(denali->dev,
> +			"IN %s: page %d is not equal to denali->page %d",
> +			__func__, page, denali->page);
> +		BUG();
> +	}
> +
> +	setup_ecc_for_xfer(denali, false, oob_required ? true : false);
> +	denali_enable_dma(denali, true);
> +
> +	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
> +
> +	clear_interrupts(denali);
> +	denali_setup_dma(denali, DENALI_READ);
> +
> +	/* wait for operation to complete */
> +	wait_for_irq(denali, irq_mask);
> +
> +	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
> +
> +	denali_enable_dma(denali, false);
> +
> +	memcpy(buf, denali->buf.buf, mtd->writesize);
> +	if (oob_required)
> +		memcpy(chip->oob_poi, denali->buf.buf + mtd->writesize,
> +		       mtd->oobsize);
> +
> +	return 0;
> +}
> +
>  static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  			    u8 *buf, int oob_required, int page)
>  {
> @@ -1182,44 +1220,6 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	return max_bitflips;
>  }
>  
> -static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> -				u8 *buf, int oob_required, int page)
> -{
> -	struct denali_nand_info *denali = mtd_to_denali(mtd);
> -	dma_addr_t addr = denali->buf.dma_buf;
> -	size_t size = mtd->writesize + mtd->oobsize;
> -	u32 irq_mask = INTR_STATUS__DMA_CMD_COMP;
> -
> -	if (page != denali->page) {
> -		dev_err(denali->dev,
> -			"IN %s: page %d is not equal to denali->page %d",
> -			__func__, page, denali->page);
> -		BUG();
> -	}
> -
> -	setup_ecc_for_xfer(denali, false, oob_required ? true : false);
> -	denali_enable_dma(denali, true);
> -
> -	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
> -
> -	clear_interrupts(denali);
> -	denali_setup_dma(denali, DENALI_READ);
> -
> -	/* wait for operation to complete */
> -	wait_for_irq(denali, irq_mask);
> -
> -	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
> -
> -	denali_enable_dma(denali, false);
> -
> -	memcpy(buf, denali->buf.buf, mtd->writesize);
> -	if (oob_required)
> -		memcpy(chip->oob_poi, denali->buf.buf + mtd->writesize,
> -		       mtd->oobsize);
> -
> -	return 0;
> -}
> -
>  static u8 denali_read_byte(struct mtd_info *mtd)
>  {
>  	struct denali_nand_info *denali = mtd_to_denali(mtd);

[toc] | [prev] | [next] | [standalone]


#1532994 — Re: [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw() above denali_read_page()

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-11-30 07:20 +0100
SubjectRe: [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw() above denali_read_page()
Message-ID<sJ6pj-2st-7@gated-at.bofh.it>
In reply to#1530895
Hi Boris,

2016-11-28 1:10 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> On Sun, 27 Nov 2016 03:06:04 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
>> This will be needed in the next commit to call denali_read_page_raw()
>> from denali_read_page().
>
> Please squash this change into patch 19. It's clearly useless to
> dissociate them.
>

I will.

But as I mentioned in the reply to 19/39,
I am thinking that another solution may be possible.

In that case, both 18/39 and 19/39 will be unnecessary.


-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]


Page 1 of 4  [1] 2 3 4  Next page →

Back to top | Article view | linux.kernel


csiph-web