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

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 3 of 4 — ← Prev page 1 2 [3] 4  Next page →

#1536457 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

On 12/05/2016 09:51 PM, Dinh Nguyen wrote:
> On Sun, Dec 4, 2016 at 10:22 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 12/05/2016 05:10 AM, Masahiro Yamada wrote:
>>> Hi Marek,
>>>
>>>
>>> 2016-12-05 12:44 GMT+09:00 Marek Vasut <marek.vasut@gmail.com>:
>>>> On 12/05/2016 04:30 AM, Masahiro Yamada wrote:
>>>>> Hi Dinh,
>>>>>
>>>>>
>>>>> 2016-12-04 7:08 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
>>>>>> Hi,
>>>>>>
>>>>>> On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>>> On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
>>>>>>>> Hi Rob,
>>>>>>>
>>>>>>> Hi!
>>>>>>>
>>>>>>>> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> (Plan A)
>>>>>>>>>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>>>>>>>>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>>>>>>>>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>>>>>>>>>
>>>>>>>>>> (Plan B)
>>>>>>>>>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>>>>>>>>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>>>>>>>>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
>>>>>>>>
>>>>>>>>> Let the Altera folks worry about their stuff. At least for soft IP in
>>>>>>>>> FPGA, it's a bit of a special case. The old string can remain as bad
>>>>>>>>> as it is.
>>>>>>>>
>>>>>>>>
>>>>>>>> Hmm, I am not sure if this IP would fit in FPGA
>>>>>>>> (to use it along with NIOS-II?)
>>>>>>>>
>>>>>>>> (even if it happened, nothing of this IP would be customizable on users' side.
>>>>>>>> When buying the IP, SoC vendors submit a list of desired features.
>>>>>>>> Denali (now Cadence) generates the RTL according to the configuration sheet.
>>>>>>>> The function is fixed at this point. So, generic compatible would be
>>>>>>>> useless anyway.)
>>>>>>>>
>>>>>>>>
>>>>>>>> If we are talking about SOCFPGA,
>>>>>>>> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
>>>>>>>> It consists of two parts:
>>>>>>>> [1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
>>>>>>>> USB, SD, NAND, ...)
>>>>>>>> [2] FPGA part (User design logic)
>>>>>>>>
>>>>>>>> The Denali NAND controller is included in [1].
>>>>>>>> So, as far as we talk about the Denali on SOCFPGA,
>>>>>>>> it is as hard-wired as Intel, Socionext's ones.
>>>>>>>
>>>>>>> That's correct, the Denali NAND IP in altera socfpga is a hardware
>>>>>>> block. You can make it available to the fabric too, but by default
>>>>>>> it's used by the ARM part of the chip, so for this discussion, you
>>>>>>> can forget that the FPGA part exists altogether.
>>>>>>>
>>>>>>> I would be in favor of plan B, since it seems to be the more often
>>>>>>> taken approach. A nice example is ci-hdrc:
>>>>>>>
>>>>>>> $ git grep compatible drivers/usb/chipidea/
>>>>>>>
>>>>>>>>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>>>>>>>>> The fact that it is denali is part of the documentation.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Let me think about this.
>>>>>>>>
>>>>>>>> Socionext bought two version of Denali IP,
>>>>>>>> and we are now re-using the newer one (v5b) for several SoCs.
>>>>>>>> Socionext has some more product lines other than Uniphier SoC family,
>>>>>>>> perhaps wider re-use might happen in the future.
>>>>>>>>
>>>>>>>> At first, I included "uniphier" in compatible, but I am still wondering
>>>>>>>> if such a specific string is good or not.
>>>>>>>>
>>>>>>>> Also, comments from Altera engineers are appreciated.
>>>>>>
>>>>>> Sorry, it's taken me a while to add comments. My altera email is very spotty now
>>>>>> that the Intel merge is completed. Please use dinguyen@kernel.org for any future
>>>>>> communications.
>>>>>>
>>>>>> Yes, everything that is said so far for the NAND controller on the
>>>>>> SoCFPGA is correct. I added the binding for the controller a while
>>>>>> back, but unfortunately, we never added the NAND interface to the
>>>>>> devkit, so we did not do much in terms of enabling it.
>>>>>>
>>>>>> I think the only SoCFPGA board I know that has the NAND interface active is
>>>>>> the TRCom board, but I have never seen that board.
>>>>>>
>>>>>> I don't have any strong opinions on this matter, just as long as the
>>>>>> original binding
>>>>>> "denali,denali-nand-dt" is kept, and I think Rob was ok with keeping
>>>>>> that binding.
>>>>>>
>>>>>
>>>>> I am proposing to add "altera,denali-nand" for Altera.
>>>>> For what, do you need the generic compatible?
>>>>> This IP has no default for it to fallback to.
>>>>
>>>> IMO just for compatibility reasons with old DTs .
>>>
>>> We generally contribute for
>>> a "working driver" (at least, should be functional to some extent)
>>> and "DT binding" bundled together.
>>>
>>> However, Altera upstreamed the DT binding first
>>> (then some parts of the DT binding turned out wrong),
>>> but they did not upstream needed driver changes in the end.
>>>
>>> So, the mainline driver has never worked on SOCFPGA, right?
>>
>> Most likely it never worked, yes.
>>
> 
> Right, looking through our downstream support, we may need to upstream a
> few changes to make upstream driver work on SoCFPGA.
> 
>>> Removing "denali,denali-nand-dt" is not breakage at all,
>>> so I do not owe anything to them, right?
>>
>> I don't think I'm really qualified to answer this one. But, there is
>> drivers/mtd/nand/denali_dt.c , which handles this compatible string
>> and it's documented in
>> Documentation/devicetree/bindings/mtd/denali-nand.txt, so doesn't that
>> make it part of the ABI ? I think we should
>> at least keep it as a fallback, that should be pretty harmless.
>>
> 
> I would like to propose "altr,denali-nand" as the binding we use to support the
> driver going forward on SoCFPGA hardware. It's pretty much the same as
> "altera,denali-nand", just with the correct vendor prefix.

Ah right, altr is the right prefix, thanks for pointing that out.
Still, wouldn't altr,socfpga-denali-nand be better ? I know it's
long, but it encodes the chip type , like ie. fsl,imx6q-usb .

> If we can please keep, "denali,denali-nand-dt" only because SoCFPGA is using
> this binding downstream, but I know that is a weak argument.

-- 
Best regards,
Marek Vasut

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

#1536492 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

On 12/05/2016 03:29 PM, Marek Vasut wrote:
> On 12/05/2016 09:51 PM, Dinh Nguyen wrote:
>> On Sun, Dec 4, 2016 at 10:22 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>> On 12/05/2016 05:10 AM, Masahiro Yamada wrote:
>>>> Hi Marek,
>>>>
>>>>
>>>> 2016-12-05 12:44 GMT+09:00 Marek Vasut <marek.vasut@gmail.com>:
>>>>> On 12/05/2016 04:30 AM, Masahiro Yamada wrote:
>>>>>> Hi Dinh,
>>>>>>
>>>>>>
>>>>>> 2016-12-04 7:08 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>>>> On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
>>>>>>>>> Hi Rob,
>>>>>>>>
>>>>>>>> Hi!
>>>>>>>>
>>>>>>>>> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> (Plan A)
>>>>>>>>>>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>>>>>>>>>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>>>>>>>>>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>>>>>>>>>>
>>>>>>>>>>> (Plan B)
>>>>>>>>>>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>>>>>>>>>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>>>>>>>>>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
>>>>>>>>>
>>>>>>>>>> Let the Altera folks worry about their stuff. At least for soft IP in
>>>>>>>>>> FPGA, it's a bit of a special case. The old string can remain as bad
>>>>>>>>>> as it is.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hmm, I am not sure if this IP would fit in FPGA
>>>>>>>>> (to use it along with NIOS-II?)
>>>>>>>>>
>>>>>>>>> (even if it happened, nothing of this IP would be customizable on users' side.
>>>>>>>>> When buying the IP, SoC vendors submit a list of desired features.
>>>>>>>>> Denali (now Cadence) generates the RTL according to the configuration sheet.
>>>>>>>>> The function is fixed at this point. So, generic compatible would be
>>>>>>>>> useless anyway.)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> If we are talking about SOCFPGA,
>>>>>>>>> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
>>>>>>>>> It consists of two parts:
>>>>>>>>> [1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
>>>>>>>>> USB, SD, NAND, ...)
>>>>>>>>> [2] FPGA part (User design logic)
>>>>>>>>>
>>>>>>>>> The Denali NAND controller is included in [1].
>>>>>>>>> So, as far as we talk about the Denali on SOCFPGA,
>>>>>>>>> it is as hard-wired as Intel, Socionext's ones.
>>>>>>>>
>>>>>>>> That's correct, the Denali NAND IP in altera socfpga is a hardware
>>>>>>>> block. You can make it available to the fabric too, but by default
>>>>>>>> it's used by the ARM part of the chip, so for this discussion, you
>>>>>>>> can forget that the FPGA part exists altogether.
>>>>>>>>
>>>>>>>> I would be in favor of plan B, since it seems to be the more often
>>>>>>>> taken approach. A nice example is ci-hdrc:
>>>>>>>>
>>>>>>>> $ git grep compatible drivers/usb/chipidea/
>>>>>>>>
>>>>>>>>>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>>>>>>>>>> The fact that it is denali is part of the documentation.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Let me think about this.
>>>>>>>>>
>>>>>>>>> Socionext bought two version of Denali IP,
>>>>>>>>> and we are now re-using the newer one (v5b) for several SoCs.
>>>>>>>>> Socionext has some more product lines other than Uniphier SoC family,
>>>>>>>>> perhaps wider re-use might happen in the future.
>>>>>>>>>
>>>>>>>>> At first, I included "uniphier" in compatible, but I am still wondering
>>>>>>>>> if such a specific string is good or not.
>>>>>>>>>
>>>>>>>>> Also, comments from Altera engineers are appreciated.
>>>>>>>
>>>>>>> Sorry, it's taken me a while to add comments. My altera email is very spotty now
>>>>>>> that the Intel merge is completed. Please use dinguyen@kernel.org for any future
>>>>>>> communications.
>>>>>>>
>>>>>>> Yes, everything that is said so far for the NAND controller on the
>>>>>>> SoCFPGA is correct. I added the binding for the controller a while
>>>>>>> back, but unfortunately, we never added the NAND interface to the
>>>>>>> devkit, so we did not do much in terms of enabling it.
>>>>>>>
>>>>>>> I think the only SoCFPGA board I know that has the NAND interface active is
>>>>>>> the TRCom board, but I have never seen that board.
>>>>>>>
>>>>>>> I don't have any strong opinions on this matter, just as long as the
>>>>>>> original binding
>>>>>>> "denali,denali-nand-dt" is kept, and I think Rob was ok with keeping
>>>>>>> that binding.
>>>>>>>
>>>>>>
>>>>>> I am proposing to add "altera,denali-nand" for Altera.
>>>>>> For what, do you need the generic compatible?
>>>>>> This IP has no default for it to fallback to.
>>>>>
>>>>> IMO just for compatibility reasons with old DTs .
>>>>
>>>> We generally contribute for
>>>> a "working driver" (at least, should be functional to some extent)
>>>> and "DT binding" bundled together.
>>>>
>>>> However, Altera upstreamed the DT binding first
>>>> (then some parts of the DT binding turned out wrong),
>>>> but they did not upstream needed driver changes in the end.
>>>>
>>>> So, the mainline driver has never worked on SOCFPGA, right?
>>>
>>> Most likely it never worked, yes.
>>>
>>
>> Right, looking through our downstream support, we may need to upstream a
>> few changes to make upstream driver work on SoCFPGA.
>>
>>>> Removing "denali,denali-nand-dt" is not breakage at all,
>>>> so I do not owe anything to them, right?
>>>
>>> I don't think I'm really qualified to answer this one. But, there is
>>> drivers/mtd/nand/denali_dt.c , which handles this compatible string
>>> and it's documented in
>>> Documentation/devicetree/bindings/mtd/denali-nand.txt, so doesn't that
>>> make it part of the ABI ? I think we should
>>> at least keep it as a fallback, that should be pretty harmless.
>>>
>>
>> I would like to propose "altr,denali-nand" as the binding we use to support the
>> driver going forward on SoCFPGA hardware. It's pretty much the same as
>> "altera,denali-nand", just with the correct vendor prefix.
> 
> Ah right, altr is the right prefix, thanks for pointing that out.
> Still, wouldn't altr,socfpga-denali-nand be better ? I know it's
> long, but it encodes the chip type , like ie. fsl,imx6q-usb .
> 

Yes, that's fine.

Dinh

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

#1530741 — [PATCH 12/39] mtd: nand: denali: return 0 for uncorrectable ECC error

When uncorrectable ECC error happens, whether the page is erased or
not, the ecc.read_page() should return zero.

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

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

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index ae9a8d2..a6445d9 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1160,6 +1160,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 		if (!is_erased(buf, mtd->writesize) ||
 		    !is_erased(chip->oob_poi, mtd->oobsize))
 			mtd->ecc_stats.failed++;
+		return 0;
 	}
 	return max_bitflips;
 }
-- 
2.7.4

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

#1530742 — [PATCH 31/39] mtd: nand: denali: remove meaningless writes to read-only registers

The write accesses to LOGICAL_PAGE_{DATA,SPARE}_SIZE have no effect
because the Denali User's Guide says these registers are read-only.

The hardware automatically multiplies the main/spare size by the
number of devices and update LOGICAL_PAGE_{DATA,SPARE}_SIZE.

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

 drivers/mtd/nand/denali.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 4b2bbcb..9193331 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -342,8 +342,6 @@ static void get_samsung_nand_para(struct denali_nand_info *denali, u8 device_id)
 
 static void get_toshiba_nand_para(struct denali_nand_info *denali)
 {
-	u32 tmp;
-
 	/*
 	 * Workaround to fix a controller bug which reports a wrong
 	 * spare area size for some kind of Toshiba NAND device
@@ -351,10 +349,6 @@ static void get_toshiba_nand_para(struct denali_nand_info *denali)
 	if ((ioread32(denali->flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
 		(ioread32(denali->flash_reg + DEVICE_SPARE_AREA_SIZE) == 64)) {
 		iowrite32(216, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
-		tmp = ioread32(denali->flash_reg + DEVICES_CONNECTED) *
-			ioread32(denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
-		iowrite32(tmp,
-				denali->flash_reg + LOGICAL_PAGE_SPARE_SIZE);
 #if SUPPORT_15BITECC
 		iowrite32(15, denali->flash_reg + ECC_CORRECTION);
 #elif SUPPORT_8BITECC
@@ -365,22 +359,12 @@ static void get_toshiba_nand_para(struct denali_nand_info *denali)
 
 static void get_hynix_nand_para(struct denali_nand_info *denali, u8 device_id)
 {
-	u32 main_size, spare_size;
-
 	switch (device_id) {
 	case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
 	case 0xD7: /* Hynix H27UDG8VEM, H27UCG8UDM or H27UCG8V5A */
 		iowrite32(128, denali->flash_reg + PAGES_PER_BLOCK);
 		iowrite32(4096, denali->flash_reg + DEVICE_MAIN_AREA_SIZE);
 		iowrite32(224, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
-		main_size = 4096 *
-			ioread32(denali->flash_reg + DEVICES_CONNECTED);
-		spare_size = 224 *
-			ioread32(denali->flash_reg + DEVICES_CONNECTED);
-		iowrite32(main_size,
-				denali->flash_reg + LOGICAL_PAGE_DATA_SIZE);
-		iowrite32(spare_size,
-				denali->flash_reg + LOGICAL_PAGE_SPARE_SIZE);
 		iowrite32(0, denali->flash_reg + DEVICE_WIDTH);
 #if SUPPORT_15BITECC
 		iowrite32(15, denali->flash_reg + ECC_CORRECTION);
-- 
2.7.4

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

#1530744 — [PATCH 32/39] mtd: nand: denali: remove unnecessary writes to ECC_CORRECTION

Because SUPPORT_15BITECC is defined, the following is dead code:

  #elif SUPPORT_8BITECC
          iowrite32(8, denali->flash_reg + ECC_CORRECTION);
  #endif

Such ifdefs are useless and unacceptable coding style.

These writes are not needed in the first place since ECC_CORRECTION
is set up by the nand_init() function.

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

 drivers/mtd/nand/denali.c | 15 +--------------
 drivers/mtd/nand/denali.h |  3 ---
 2 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 9193331..63f7500 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -62,8 +62,6 @@ MODULE_PARM_DESC(onfi_timing_mode,
  */
 #define CHIP_SELECT_INVALID	-1
 
-#define SUPPORT_8BITECC		1
-
 /*
  * This macro divides two integers and rounds fractional values up
  * to the nearest integer value.
@@ -347,14 +345,8 @@ static void get_toshiba_nand_para(struct denali_nand_info *denali)
 	 * spare area size for some kind of Toshiba NAND device
 	 */
 	if ((ioread32(denali->flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
-		(ioread32(denali->flash_reg + DEVICE_SPARE_AREA_SIZE) == 64)) {
+		(ioread32(denali->flash_reg + DEVICE_SPARE_AREA_SIZE) == 64))
 		iowrite32(216, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
-#if SUPPORT_15BITECC
-		iowrite32(15, denali->flash_reg + ECC_CORRECTION);
-#elif SUPPORT_8BITECC
-		iowrite32(8, denali->flash_reg + ECC_CORRECTION);
-#endif
-	}
 }
 
 static void get_hynix_nand_para(struct denali_nand_info *denali, u8 device_id)
@@ -366,11 +358,6 @@ static void get_hynix_nand_para(struct denali_nand_info *denali, u8 device_id)
 		iowrite32(4096, denali->flash_reg + DEVICE_MAIN_AREA_SIZE);
 		iowrite32(224, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
 		iowrite32(0, denali->flash_reg + DEVICE_WIDTH);
-#if SUPPORT_15BITECC
-		iowrite32(15, denali->flash_reg + ECC_CORRECTION);
-#elif SUPPORT_8BITECC
-		iowrite32(8, denali->flash_reg + ECC_CORRECTION);
-#endif
 		break;
 	default:
 		dev_warn(denali->dev,
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index e3fe3bc..d621b74 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -387,9 +387,6 @@
 #define CLK_X  5
 #define CLK_MULTI 4
 
-#define SUPPORT_15BITECC        1
-#define SUPPORT_8BITECC         1
-
 #define ONFI_BLOOM_TIME         1
 #define MODE5_WORKAROUND        0
 
-- 
2.7.4

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

#1530745 — [PATCH 01/39] mtd: nand: allow to set only one of ECC size and ECC strength from DT

Currently, it is valid to specify both "nand-ecc-step-size" and
"nand-ecc-strength", but not allowed to set only one of them.

This requirement has a conflict with "nand-ecc-maximize"; this flag
is used when you want the driver to choose the best ECC strength.
If "nand-ecc-maximize" is set, "nand-ecc-strength" is very likely to
be unset.

It would be possible to make the if-conditional more complex by
adding the check for the NAND_ECC_MAXIMIZE flag, but I chose to drop
the check entirely.  I thought of the situation where the hardware
has a fixed ECC step size (so it can be hard-coded in the driver),
whereas the ECC strength is configurable by software.  In that case,
we may want to only set "nand-ecc-strength" (or "nand-ecc-maximize")
in DT.

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

The Denali NAND is the case.

The ecc.size is fixed when the RTL is delivered, while the
driver can choose ecc.strength from some supported values.

 For Intel and Altera, available ecc.strength are 8, 15.
 For Socionext UniPhier, available ecc.strength are 8, 16, 24.


 drivers/mtd/nand/nand_base.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 8c74d8c..c1ed91d 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4266,12 +4266,6 @@ static int nand_dt_init(struct nand_chip *chip)
 	ecc_strength = of_get_nand_ecc_strength(dn);
 	ecc_step = of_get_nand_ecc_step_size(dn);
 
-	if ((ecc_step >= 0 && !(ecc_strength >= 0)) ||
-	    (!(ecc_step >= 0) && ecc_strength >= 0)) {
-		pr_err("must set both strength and step size in DT\n");
-		return -EINVAL;
-	}
-
 	if (ecc_mode >= 0)
 		chip->ecc.mode = ecc_mode;
 
-- 
2.7.4

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

#1530877 — Re: [PATCH 01/39] mtd: nand: allow to set only one of ECC size and ECC strength from DT

On Sun, 27 Nov 2016 03:05:47 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Currently, it is valid to specify both "nand-ecc-step-size" and
> "nand-ecc-strength", but not allowed to set only one of them.
> 
> This requirement has a conflict with "nand-ecc-maximize"; this flag
> is used when you want the driver to choose the best ECC strength.
> If "nand-ecc-maximize" is set, "nand-ecc-strength" is very likely to
> be unset.

Well, when I added nand-ecc-maximize I was assuming that the driver
would choose both step-size and strength to maximize the ECC strength
on the whole page, but you're right, we might need fine-grained
tweaking (forcing the step-size but letting the driver choose the ECC
strength).

> 
> It would be possible to make the if-conditional more complex by
> adding the check for the NAND_ECC_MAXIMIZE flag, but I chose to drop
> the check entirely.  I thought of the situation where the hardware
> has a fixed ECC step size (so it can be hard-coded in the driver),
> whereas the ECC strength is configurable by software.  In that case,
> we may want to only set "nand-ecc-strength" (or "nand-ecc-maximize")
> in DT.

I would also add that checking things at this level is not really
relevant. The driver is likely to check the ecc.size and ecc.strength
values anyway, and pick a default value (chip->nand_ecc_xx_ds or some
hard-coded values if the ECC engine only support one specific case) if
one of them is unassigned (left to 0).

I'll let some time for others to review before queuing this patch for
4.11.

> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
> The Denali NAND is the case.
> 
> The ecc.size is fixed when the RTL is delivered, while the
> driver can choose ecc.strength from some supported values.
> 
>  For Intel and Altera, available ecc.strength are 8, 15.
>  For Socionext UniPhier, available ecc.strength are 8, 16, 24.
> 
> 
>  drivers/mtd/nand/nand_base.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 8c74d8c..c1ed91d 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -4266,12 +4266,6 @@ static int nand_dt_init(struct nand_chip *chip)
>  	ecc_strength = of_get_nand_ecc_strength(dn);
>  	ecc_step = of_get_nand_ecc_step_size(dn);
>  
> -	if ((ecc_step >= 0 && !(ecc_strength >= 0)) ||
> -	    (!(ecc_step >= 0) && ecc_strength >= 0)) {
> -		pr_err("must set both strength and step size in DT\n");
> -		return -EINVAL;
> -	}
> -
>  	if (ecc_mode >= 0)
>  		chip->ecc.mode = ecc_mode;
>  

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

#1530747 — [PATCH 06/39] mtd: nand: denali: fix write_oob_data() function

The function write_oob_data() performs write access to the spare
area, so passing "false" to the 3rd argument (transfer_spare) of
denali_send_pipeline_cmd() is weird.

Actually, the transfer_spare is ignored for MAP10 operation with
SPARE_ACCESS (0x41), so this change has no impact on the hardware
behavior.  Yet, we should keep our code to make sense.

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 61e1e33..b2b050b 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -834,7 +834,7 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 
 	denali->page = page;
 
-	if (denali_send_pipeline_cmd(denali, false, false, SPARE_ACCESS,
+	if (denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS,
 							DENALI_WRITE) == PASS) {
 		write_data_to_flash_mem(denali, buf, mtd->oobsize);
 
-- 
2.7.4

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

#1530748 — [PATCH 03/39] mtd: nand: denali: remove redundant define of BANK(x)

This macro is defined twice in denali.c (around line 103 and
line 656), so remove the second one.

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

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

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 9b3140f..61e1e33 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -653,7 +653,6 @@ static irqreturn_t denali_isr(int irq, void *dev_id)
 	spin_unlock(&denali->irq_lock);
 	return result;
 }
-#define BANK(x) ((x) << 24)
 
 static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
 {
-- 
2.7.4

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

#1530749 — [PATCH 05/39] mtd: nand: denali: fix comment of denali_nand_info::flash_mem

The same comment "Mapped io reg base address" for flash_reg and
flash_mem probably due to the mistake of copy-paste work.
Of course, the latter is not the register base address.

Reword the comments using the terminology in the Denali User's Guide.

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

 drivers/mtd/nand/denali.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 4fad43b..8bec980 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -408,8 +408,8 @@ struct denali_nand_info {
 	struct device *dev;
 	int total_used_banks;
 	uint16_t page;
-	void __iomem *flash_reg;  /* Mapped io reg base address */
-	void __iomem *flash_mem;  /* Mapped io reg base address */
+	void __iomem *flash_reg;	/* Register Interface */
+	void __iomem *flash_mem;	/* Host Data/Command Interface */
 
 	/* elements used by ISR */
 	struct completion complete;
-- 
2.7.4

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

#1530750 — [PATCH 14/39] mtd: nand: denali: replace uint{8/16/32}_t with u{8/16/32}

This will make the code shorter and avoid 80 columns wrap-around
in some places.

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

 drivers/mtd/nand/denali.c | 188 ++++++++++++++++++++++------------------------
 drivers/mtd/nand/denali.h |  12 +--
 2 files changed, 95 insertions(+), 105 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 4cc8945..a7dc692 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -104,11 +104,9 @@ static inline struct denali_nand_info *mtd_to_denali(struct mtd_info *mtd)
 
 /* forward declarations */
 static void clear_interrupts(struct denali_nand_info *denali);
-static uint32_t wait_for_irq(struct denali_nand_info *denali,
-							uint32_t irq_mask);
-static void denali_irq_enable(struct denali_nand_info *denali,
-							uint32_t int_mask);
-static uint32_t read_interrupt_status(struct denali_nand_info *denali);
+static u32 wait_for_irq(struct denali_nand_info *denali, u32 irq_mask);
+static void denali_irq_enable(struct denali_nand_info *denali, u32 int_mask);
+static u32 read_interrupt_status(struct denali_nand_info *denali);
 
 /*
  * Certain operations for the denali NAND controller use an indexed mode to
@@ -116,8 +114,7 @@ static uint32_t read_interrupt_status(struct denali_nand_info *denali);
  * of the command to the device memory followed by the data. This function
  * abstracts this common operation.
  */
-static void index_addr(struct denali_nand_info *denali,
-				uint32_t address, uint32_t data)
+static void index_addr(struct denali_nand_info *denali, u32 address, u32 data)
 {
 	iowrite32(address, denali->flash_mem);
 	iowrite32(data, denali->flash_mem + 0x10);
@@ -125,7 +122,7 @@ static void index_addr(struct denali_nand_info *denali,
 
 /* Perform an indexed read of the device */
 static void index_addr_read_data(struct denali_nand_info *denali,
-				 uint32_t address, uint32_t *pdata)
+				 u32 address, u32 *pdata)
 {
 	iowrite32(address, denali->flash_mem);
 	*pdata = ioread32(denali->flash_mem + 0x10);
@@ -140,7 +137,7 @@ static void reset_buf(struct denali_nand_info *denali)
 	denali->buf.head = denali->buf.tail = 0;
 }
 
-static void write_byte_to_buf(struct denali_nand_info *denali, uint8_t byte)
+static void write_byte_to_buf(struct denali_nand_info *denali, u8 byte)
 {
 	denali->buf.buf[denali->buf.tail++] = byte;
 }
@@ -148,7 +145,7 @@ static void write_byte_to_buf(struct denali_nand_info *denali, uint8_t byte)
 /* reads the status of the device */
 static void read_status(struct denali_nand_info *denali)
 {
-	uint32_t cmd;
+	u32 cmd;
 
 	/* initialize the data buffer to store status */
 	reset_buf(denali);
@@ -163,8 +160,8 @@ static void read_status(struct denali_nand_info *denali)
 /* resets a specific device connected to the core */
 static void reset_bank(struct denali_nand_info *denali)
 {
-	uint32_t irq_status;
-	uint32_t irq_mask = INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT;
+	u32 irq_status;
+	u32 irq_mask = INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT;
 
 	clear_interrupts(denali);
 
@@ -177,7 +174,7 @@ static void reset_bank(struct denali_nand_info *denali)
 }
 
 /* Reset the flash controller */
-static uint16_t denali_nand_reset(struct denali_nand_info *denali)
+static u16 denali_nand_reset(struct denali_nand_info *denali)
 {
 	int i;
 
@@ -208,27 +205,26 @@ static uint16_t denali_nand_reset(struct denali_nand_info *denali)
  * programs the clocking register accordingly. The mode is determined by
  * the get_onfi_nand_para routine.
  */
-static void nand_onfi_timing_set(struct denali_nand_info *denali,
-								uint16_t mode)
+static void nand_onfi_timing_set(struct denali_nand_info *denali, u16 mode)
 {
-	uint16_t Trea[6] = {40, 30, 25, 20, 20, 16};
-	uint16_t Trp[6] = {50, 25, 17, 15, 12, 10};
-	uint16_t Treh[6] = {30, 15, 15, 10, 10, 7};
-	uint16_t Trc[6] = {100, 50, 35, 30, 25, 20};
-	uint16_t Trhoh[6] = {0, 15, 15, 15, 15, 15};
-	uint16_t Trloh[6] = {0, 0, 0, 0, 5, 5};
-	uint16_t Tcea[6] = {100, 45, 30, 25, 25, 25};
-	uint16_t Tadl[6] = {200, 100, 100, 100, 70, 70};
-	uint16_t Trhw[6] = {200, 100, 100, 100, 100, 100};
-	uint16_t Trhz[6] = {200, 100, 100, 100, 100, 100};
-	uint16_t Twhr[6] = {120, 80, 80, 60, 60, 60};
-	uint16_t Tcs[6] = {70, 35, 25, 25, 20, 15};
-
-	uint16_t data_invalid_rhoh, data_invalid_rloh, data_invalid;
-	uint16_t dv_window = 0;
-	uint16_t en_lo, en_hi;
-	uint16_t acc_clks;
-	uint16_t addr_2_data, re_2_we, re_2_re, we_2_re, cs_cnt;
+	u16 Trea[6] = {40, 30, 25, 20, 20, 16};
+	u16 Trp[6] = {50, 25, 17, 15, 12, 10};
+	u16 Treh[6] = {30, 15, 15, 10, 10, 7};
+	u16 Trc[6] = {100, 50, 35, 30, 25, 20};
+	u16 Trhoh[6] = {0, 15, 15, 15, 15, 15};
+	u16 Trloh[6] = {0, 0, 0, 0, 5, 5};
+	u16 Tcea[6] = {100, 45, 30, 25, 25, 25};
+	u16 Tadl[6] = {200, 100, 100, 100, 70, 70};
+	u16 Trhw[6] = {200, 100, 100, 100, 100, 100};
+	u16 Trhz[6] = {200, 100, 100, 100, 100, 100};
+	u16 Twhr[6] = {120, 80, 80, 60, 60, 60};
+	u16 Tcs[6] = {70, 35, 25, 25, 20, 15};
+
+	u16 data_invalid_rhoh, data_invalid_rloh, data_invalid;
+	u16 dv_window = 0;
+	u16 en_lo, en_hi;
+	u16 acc_clks;
+	u16 addr_2_data, re_2_we, re_2_re, we_2_re, cs_cnt;
 
 	en_lo = CEIL_DIV(Trp[mode], CLK_X);
 	en_hi = CEIL_DIV(Treh[mode], CLK_X);
@@ -300,7 +296,7 @@ static void nand_onfi_timing_set(struct denali_nand_info *denali,
 }
 
 /* queries the NAND device to see what ONFI modes it supports. */
-static uint16_t get_onfi_nand_para(struct denali_nand_info *denali)
+static u16 get_onfi_nand_para(struct denali_nand_info *denali)
 {
 	int i;
 
@@ -330,8 +326,7 @@ static uint16_t get_onfi_nand_para(struct denali_nand_info *denali)
 	return PASS;
 }
 
-static void get_samsung_nand_para(struct denali_nand_info *denali,
-							uint8_t device_id)
+static void get_samsung_nand_para(struct denali_nand_info *denali, u8 device_id)
 {
 	if (device_id == 0xd3) { /* Samsung K9WAG08U1A */
 		/* Set timing register values according to datasheet */
@@ -347,7 +342,7 @@ static void get_samsung_nand_para(struct denali_nand_info *denali,
 
 static void get_toshiba_nand_para(struct denali_nand_info *denali)
 {
-	uint32_t tmp;
+	u32 tmp;
 
 	/*
 	 * Workaround to fix a controller bug which reports a wrong
@@ -368,10 +363,9 @@ static void get_toshiba_nand_para(struct denali_nand_info *denali)
 	}
 }
 
-static void get_hynix_nand_para(struct denali_nand_info *denali,
-							uint8_t device_id)
+static void get_hynix_nand_para(struct denali_nand_info *denali, u8 device_id)
 {
-	uint32_t main_size, spare_size;
+	u32 main_size, spare_size;
 
 	switch (device_id) {
 	case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
@@ -408,7 +402,7 @@ static void get_hynix_nand_para(struct denali_nand_info *denali,
  */
 static void find_valid_banks(struct denali_nand_info *denali)
 {
-	uint32_t id[denali->max_banks];
+	u32 id[denali->max_banks];
 	int i;
 
 	denali->total_used_banks = 1;
@@ -453,7 +447,7 @@ static void find_valid_banks(struct denali_nand_info *denali)
  */
 static void detect_max_banks(struct denali_nand_info *denali)
 {
-	uint32_t features = ioread32(denali->flash_reg + FEATURES);
+	u32 features = ioread32(denali->flash_reg + FEATURES);
 	/*
 	 * Read the revision register, so we can calculate the max_banks
 	 * properly: the encoding changed from rev 5.0 to 5.1
@@ -467,11 +461,11 @@ static void detect_max_banks(struct denali_nand_info *denali)
 		denali->max_banks = 1 << (features & FEATURES__N_BANKS);
 }
 
-static uint16_t denali_nand_timing_set(struct denali_nand_info *denali)
+static u16 denali_nand_timing_set(struct denali_nand_info *denali)
 {
-	uint16_t status = PASS;
-	uint32_t id_bytes[8], addr;
-	uint8_t maf_id, device_id;
+	u16 status = PASS;
+	u32 id_bytes[8], addr;
+	u8 maf_id, device_id;
 	int i;
 
 	/*
@@ -526,7 +520,7 @@ static uint16_t denali_nand_timing_set(struct denali_nand_info *denali)
 }
 
 static void denali_set_intr_modes(struct denali_nand_info *denali,
-					uint16_t INT_ENABLE)
+				  u16 INT_ENABLE)
 {
 	if (INT_ENABLE)
 		iowrite32(1, denali->flash_reg + GLOBAL_INT_ENABLE);
@@ -545,7 +539,7 @@ static inline bool is_flash_bank_valid(int flash_bank)
 
 static void denali_irq_init(struct denali_nand_info *denali)
 {
-	uint32_t int_mask;
+	u32 int_mask;
 	int i;
 
 	/* Disable global interrupts */
@@ -565,8 +559,7 @@ static void denali_irq_cleanup(int irqnum, struct denali_nand_info *denali)
 	denali_set_intr_modes(denali, false);
 }
 
-static void denali_irq_enable(struct denali_nand_info *denali,
-							uint32_t int_mask)
+static void denali_irq_enable(struct denali_nand_info *denali, u32 int_mask)
 {
 	int i;
 
@@ -578,16 +571,16 @@ static void denali_irq_enable(struct denali_nand_info *denali,
  * This function only returns when an interrupt that this driver cares about
  * occurs. This is to reduce the overhead of servicing interrupts
  */
-static inline uint32_t denali_irq_detected(struct denali_nand_info *denali)
+static inline u32 denali_irq_detected(struct denali_nand_info *denali)
 {
 	return read_interrupt_status(denali) & DENALI_IRQ_ALL;
 }
 
 /* Interrupts are cleared by writing a 1 to the appropriate status bit */
 static inline void clear_interrupt(struct denali_nand_info *denali,
-							uint32_t irq_mask)
+				   u32 irq_mask)
 {
-	uint32_t intr_status_reg;
+	u32 intr_status_reg;
 
 	intr_status_reg = INTR_STATUS(denali->flash_bank);
 
@@ -596,7 +589,7 @@ static inline void clear_interrupt(struct denali_nand_info *denali,
 
 static void clear_interrupts(struct denali_nand_info *denali)
 {
-	uint32_t status;
+	u32 status;
 
 	spin_lock_irq(&denali->irq_lock);
 
@@ -607,9 +600,9 @@ static void clear_interrupts(struct denali_nand_info *denali)
 	spin_unlock_irq(&denali->irq_lock);
 }
 
-static uint32_t read_interrupt_status(struct denali_nand_info *denali)
+static u32 read_interrupt_status(struct denali_nand_info *denali)
 {
-	uint32_t intr_status_reg;
+	u32 intr_status_reg;
 
 	intr_status_reg = INTR_STATUS(denali->flash_bank);
 
@@ -623,7 +616,7 @@ static uint32_t read_interrupt_status(struct denali_nand_info *denali)
 static irqreturn_t denali_isr(int irq, void *dev_id)
 {
 	struct denali_nand_info *denali = dev_id;
-	uint32_t irq_status;
+	u32 irq_status;
 	irqreturn_t result = IRQ_NONE;
 
 	spin_lock(&denali->irq_lock);
@@ -654,10 +647,10 @@ static irqreturn_t denali_isr(int irq, void *dev_id)
 	return result;
 }
 
-static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
+static u32 wait_for_irq(struct denali_nand_info *denali, u32 irq_mask)
 {
 	unsigned long comp_res;
-	uint32_t intr_status;
+	u32 intr_status;
 	unsigned long timeout = msecs_to_jiffies(1000);
 
 	do {
@@ -717,8 +710,8 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
 				    int access_type, int op)
 {
 	int status = PASS;
-	uint32_t page_count = 1;
-	uint32_t addr, cmd, irq_status, irq_mask;
+	u32 page_count = 1;
+	u32 addr, cmd, irq_status, irq_mask;
 
 	if (op == DENALI_READ)
 		irq_mask = INTR_STATUS__LOAD_COMP;
@@ -783,9 +776,9 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
 
 /* helper function that simply writes a buffer to the flash */
 static int write_data_to_flash_mem(struct denali_nand_info *denali,
-				   const uint8_t *buf, int len)
+				   const u8 *buf, int len)
 {
-	uint32_t *buf32;
+	u32 *buf32;
 	int i;
 
 	/*
@@ -795,7 +788,7 @@ static int write_data_to_flash_mem(struct denali_nand_info *denali,
 	BUG_ON((len % 4) != 0);
 
 	/* write the data to the flash memory */
-	buf32 = (uint32_t *)buf;
+	buf32 = (u32 *)buf;
 	for (i = 0; i < len / 4; i++)
 		iowrite32(*buf32++, denali->flash_mem + 0x10);
 	return i * 4; /* intent is to return the number of bytes read */
@@ -803,9 +796,9 @@ static int write_data_to_flash_mem(struct denali_nand_info *denali,
 
 /* helper function that simply reads a buffer from the flash */
 static int read_data_from_flash_mem(struct denali_nand_info *denali,
-				    uint8_t *buf, int len)
+				    u8 *buf, int len)
 {
-	uint32_t *buf32;
+	u32 *buf32;
 	int i;
 
 	/*
@@ -817,19 +810,18 @@ static int read_data_from_flash_mem(struct denali_nand_info *denali,
 	BUG_ON((len % 4) != 0);
 
 	/* transfer the data from the flash */
-	buf32 = (uint32_t *)buf;
+	buf32 = (u32 *)buf;
 	for (i = 0; i < len / 4; i++)
 		*buf32++ = ioread32(denali->flash_mem + 0x10);
 	return i * 4; /* intent is to return the number of bytes read */
 }
 
 /* writes OOB data to the device */
-static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
+static int write_oob_data(struct mtd_info *mtd, u8 *buf, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	uint32_t irq_status;
-	uint32_t irq_mask = INTR_STATUS__PROGRAM_COMP |
-						INTR_STATUS__PROGRAM_FAIL;
+	u32 irq_status;
+	u32 irq_mask = INTR_STATUS__PROGRAM_COMP | INTR_STATUS__PROGRAM_FAIL;
 	int status = 0;
 
 	denali->page = page;
@@ -853,11 +845,11 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 }
 
 /* reads OOB data from the device */
-static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
+static void read_oob_data(struct mtd_info *mtd, u8 *buf, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	uint32_t irq_mask = INTR_STATUS__LOAD_COMP;
-	uint32_t irq_status, addr, cmd;
+	u32 irq_mask = INTR_STATUS__LOAD_COMP;
+	u32 irq_status, addr, cmd;
 
 	denali->page = page;
 
@@ -893,7 +885,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
  * this function examines buffers to see if they contain data that
  * indicate that the buffer is part of an erased region of flash.
  */
-static bool is_erased(uint8_t *buf, int len)
+static bool is_erased(u8 *buf, int len)
 {
 	int i;
 
@@ -911,16 +903,16 @@ static bool is_erased(uint8_t *buf, int len)
 #define ECC_ERR_DEVICE(x)	(((x) & ERR_CORRECTION_INFO__DEVICE_NR) >> 8)
 #define ECC_LAST_ERR(x)		((x) & ERR_CORRECTION_INFO__LAST_ERR_INFO)
 
-static bool handle_ecc(struct denali_nand_info *denali, uint8_t *buf,
-		       uint32_t irq_status, unsigned int *max_bitflips)
+static bool handle_ecc(struct denali_nand_info *denali, u8 *buf,
+		       u32 irq_status, unsigned int *max_bitflips)
 {
 	bool check_erased_page = false;
 	unsigned int bitflips = 0;
 
 	if (irq_status & INTR_STATUS__ECC_ERR) {
 		/* read the ECC errors. we'll ignore them for now */
-		uint32_t err_address, err_correction_info, err_byte,
-			 err_sector, err_device, err_correction_value;
+		u32 err_address, err_correction_info, err_byte,
+			err_sector, err_device, err_correction_value;
 		denali_set_intr_modes(denali, false);
 
 		do {
@@ -993,9 +985,9 @@ static void denali_enable_dma(struct denali_nand_info *denali, bool en)
 /* setups the HW to perform the data DMA */
 static void denali_setup_dma(struct denali_nand_info *denali, int op)
 {
-	uint32_t mode;
+	u32 mode;
 	const int page_count = 1;
-	uint32_t addr = denali->buf.dma_buf;
+	u32 addr = denali->buf.dma_buf;
 
 	mode = MODE_10 | BANK(denali->flash_bank);
 
@@ -1019,14 +1011,13 @@ static void denali_setup_dma(struct denali_nand_info *denali, int op)
  * configuration details.
  */
 static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf, bool raw_xfer)
+			const u8 *buf, bool raw_xfer)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 	dma_addr_t addr = denali->buf.dma_buf;
 	size_t size = mtd->writesize + mtd->oobsize;
-	uint32_t irq_status;
-	uint32_t irq_mask = INTR_STATUS__DMA_CMD_COMP |
-						INTR_STATUS__PROGRAM_FAIL;
+	u32 irq_status;
+	u32 irq_mask = INTR_STATUS__DMA_CMD_COMP | INTR_STATUS__PROGRAM_FAIL;
 
 	/*
 	 * if it is a raw xfer, we want to disable ecc and send the spare area.
@@ -1075,7 +1066,7 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
  * by write_page above.
  */
 static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
-				const uint8_t *buf, int oob_required, int page)
+				const u8 *buf, int oob_required, int page)
 {
 	/*
 	 * for regular page writes, we let HW handle all the ECC
@@ -1090,7 +1081,7 @@ static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
  * write_page() function above.
  */
 static int denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-				 const uint8_t *buf, int oob_required,
+				 const u8 *buf, int oob_required,
 				 int page)
 {
 	/*
@@ -1115,7 +1106,7 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
-			    uint8_t *buf, int oob_required, int page)
+			    u8 *buf, int oob_required, int page)
 {
 	unsigned int max_bitflips;
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
@@ -1123,9 +1114,8 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	dma_addr_t addr = denali->buf.dma_buf;
 	size_t size = mtd->writesize + mtd->oobsize;
 
-	uint32_t irq_status;
-	uint32_t irq_mask = INTR_STATUS__ECC_TRANSACTION_DONE |
-			    INTR_STATUS__ECC_ERR;
+	u32 irq_status;
+	u32 irq_mask = INTR_STATUS__ECC_TRANSACTION_DONE | INTR_STATUS__ECC_ERR;
 	bool check_erased_page = false;
 
 	if (page != denali->page) {
@@ -1169,12 +1159,12 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int oob_required, int page)
+				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;
-	uint32_t irq_mask = INTR_STATUS__DMA_CMD_COMP;
+	u32 irq_mask = INTR_STATUS__DMA_CMD_COMP;
 
 	if (page != denali->page) {
 		dev_err(denali->dev,
@@ -1206,10 +1196,10 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	return 0;
 }
 
-static uint8_t denali_read_byte(struct mtd_info *mtd)
+static u8 denali_read_byte(struct mtd_info *mtd)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	uint8_t result = 0xff;
+	u8 result = 0xff;
 
 	if (denali->buf.head < denali->buf.tail)
 		result = denali->buf.buf[denali->buf.head++];
@@ -1240,7 +1230,7 @@ static int denali_erase(struct mtd_info *mtd, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
-	uint32_t cmd, irq_status;
+	u32 cmd, irq_status;
 
 	clear_interrupts(denali);
 
@@ -1259,7 +1249,7 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
 			   int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	uint32_t addr, id;
+	u32 addr, id;
 	int i;
 
 	switch (cmd) {
@@ -1370,8 +1360,8 @@ static const struct mtd_ooblayout_ops denali_ooblayout_ops = {
 	.free = denali_ooblayout_free,
 };
 
-static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
-static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
+static u8 bbt_pattern[] = {'B', 'b', 't', '0' };
+static u8 mirror_pattern[] = {'1', 't', 'b', 'B' };
 
 static struct nand_bbt_descr bbt_main_descr = {
 	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 95ed32e..69314d0 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -391,7 +391,7 @@
 struct nand_buf {
 	int head;
 	int tail;
-	uint8_t *buf;
+	u8 *buf;
 	dma_addr_t dma_buf;
 };
 
@@ -407,19 +407,19 @@ struct denali_nand_info {
 	struct nand_buf buf;
 	struct device *dev;
 	int total_used_banks;
-	uint16_t page;
+	u16 page;
 	void __iomem *flash_reg;	/* Register Interface */
 	void __iomem *flash_mem;	/* Host Data/Command Interface */
 
 	/* elements used by ISR */
 	struct completion complete;
 	spinlock_t irq_lock;
-	uint32_t irq_status;
+	u32 irq_status;
 	int irq;
 
-	uint32_t devnum;	/* represent how many nands connected */
-	uint32_t bbtskipbytes;
-	uint32_t max_banks;
+	u32 devnum;	/* represent how many nands connected */
+	u32 bbtskipbytes;
+	u32 max_banks;
 	unsigned int caps;
 };
 
-- 
2.7.4

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

#1530752 — [PATCH 24/39] mtd: nand: denali: add NEW_N_BANKS_FORMAT capability

Commit 271707b1d817 ("mtd: nand: denali: max_banks calculation
changed in revision 5.1") added a revision check to support the
new max_banks encoding.  Its git-log states "The encoding of
max_banks changed in Denali revision 5.1" but I doubt it.

The revision register on some UniPhier SoCs says the IP is 5.0
but the max_banks is encoded in the new format.  The revision of
this IP is often useless.

In order to provide a way to calculate correct max_banks without
relying on the revision register, add DENALI_CAPS_NEW_N_BANKS_FORMAT
capability (quirk).

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

 drivers/mtd/nand/denali.c | 24 +++++++++++++++++-------
 drivers/mtd/nand/denali.h |  1 +
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 752ad98..614b4a5 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -448,17 +448,27 @@ static void find_valid_banks(struct denali_nand_info *denali)
 static void detect_max_banks(struct denali_nand_info *denali)
 {
 	u32 features = ioread32(denali->flash_reg + FEATURES);
+	bool old_format;
+
 	/*
-	 * Read the revision register, so we can calculate the max_banks
-	 * properly: the encoding changed from rev 5.0 to 5.1
+	 * There are some IP versions with different n_banks encoding.
+	 * Some people say the change happened from rev 5.0 to 5.1, while
+	 * there exist variants with revision older than 5.1 but new encoding.
+	 * The option flag is available in case the revision is useless.
 	 */
-	u32 revision = MAKE_COMPARABLE_REVISION(
+	if (denali->caps & DENALI_CAPS_NEW_N_BANKS_FORMAT)
+		old_format = false;
+	else {
+		u32 revision = MAKE_COMPARABLE_REVISION(
 				ioread32(denali->flash_reg + REVISION));
 
-	if (revision < REVISION_5_1)
-		denali->max_banks = 2 << (features & FEATURES__N_BANKS);
-	else
-		denali->max_banks = 1 << (features & FEATURES__N_BANKS);
+		old_format = revision < REVISION_5_1;
+	}
+
+	denali->max_banks = 1 << (features & FEATURES__N_BANKS);
+
+	if (old_format)
+		denali->max_banks <<= 1;
 }
 
 static u16 denali_nand_timing_set(struct denali_nand_info *denali)
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 9bdf037..e3fe3bc 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -436,6 +436,7 @@ struct denali_nand_info {
 	unsigned int caps;
 #define DENALI_CAPS_HW_ECC_FIXUP		BIT(0)
 #define DENALI_CAPS_DMA_64BIT			BIT(1)
+#define DENALI_CAPS_NEW_N_BANKS_FORMAT		BIT(2)
 };
 
 extern int denali_init(struct denali_nand_info *denali);
-- 
2.7.4

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

#1530753 — [PATCH 26/39] mtd: nand: denali: call nand_set_flash_node() to set DT node

This will allow nand_dt_init() to parse DT properties in the NAND
controller device node.

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

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

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 07de879..0f94381b 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1510,6 +1510,7 @@ int denali_init(struct denali_nand_info *denali)
 	/* now that our ISR is registered, we can enable interrupts */
 	denali_set_intr_modes(denali, true);
 	mtd->name = "denali-nand";
+	nand_set_flash_node(chip, denali->dev->of_node);
 
 	/* register the driver with the NAND core subsystem */
 	chip->select_chip = denali_select_chip;
-- 
2.7.4

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

#1530754 — [PATCH 33/39] mtd: nand: denali: support 1024 byte ECC step size

This driver was originally written for the Intel MRST platform with
several platform specific parameters hard-coded.  Another thing we
need to fix is the hard-coded ECC step size.  Currently, it is
defined as follows:

  #define ECC_SECTOR_SIZE 512

(somehow, it is defined in both denali.c and denali.h)

This must be avoided because the Denali IP supports 1024 byte ECC
size as well.  Add a new flag DENALI_CAPS_ECC_SIZE_1024. If it is
specified, ecc.size is set to 1024, otherwise set to 512.

We can use "nand-ecc-step-size" DT property to override the ecc.size
if we want, but this capability flag can provide the reasonable
default because it is associated with the DT compatible strings.

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

 .../devicetree/bindings/mtd/denali-nand.txt        |  4 ++++
 drivers/mtd/nand/denali.c                          | 26 +++++++++++-----------
 drivers/mtd/nand/denali.h                          |  3 +--
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index 603110b..e9d5818 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -6,6 +6,10 @@ Required properties:
   - reg-names: Should contain the reg names "nand_data" and "denali_reg"
   - interrupts : The interrupt number.
 
+Optional properties:
+  - nand-ecc-step-size: must be 512 or 1024.  If not specified, default to 512.
+    see nand.txt for details.
+
 The device tree may optionally contain sub-nodes describing partitions of the
 address space. See partition.txt for more detail.
 
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 63f7500..5d80f16 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -894,8 +894,6 @@ static bool denali_hw_ecc_fixup(struct denali_nand_info *denali,
 	return false;
 }
 
-#define ECC_SECTOR_SIZE 512
-
 #define ECC_SECTOR(x)	(((x) & ECC_ERROR_ADDRESS__SECTOR_NR) >> 12)
 #define ECC_BYTE(x)	(((x) & ECC_ERROR_ADDRESS__OFFSET))
 #define ECC_CORRECTION_VALUE(x) ((x) & ERR_CORRECTION_INFO__BYTEMASK)
@@ -908,6 +906,7 @@ static bool denali_sw_ecc_fixup(struct denali_nand_info *denali, u8 *buf,
 {
 	bool check_erased_page = false;
 	unsigned int bitflips = 0;
+	unsigned int ecc_size = denali->nand.ecc.size;
 	u32 err_address, err_correction_info, err_byte, err_sector, err_device,
 	    err_correction_value;
 
@@ -930,18 +929,18 @@ static bool denali_sw_ecc_fixup(struct denali_nand_info *denali, u8 *buf,
 
 		if (ECC_ERROR_CORRECTABLE(err_correction_info)) {
 			/*
-			 * If err_byte is larger than ECC_SECTOR_SIZE, means error
+			 * If err_byte is larger than ecc_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) {
+			if (err_byte < ecc_size) {
 				struct mtd_info *mtd =
 					nand_to_mtd(&denali->nand);
 				int offset;
 
-				offset = (err_sector * ECC_SECTOR_SIZE + err_byte) *
+				offset = (err_sector * ecc_size + err_byte) *
 					denali->devnum + err_device;
 				/* correct the ECC error */
 				buf[offset] ^= err_correction_value;
@@ -1590,22 +1589,25 @@ int denali_init(struct denali_nand_info *denali)
 	/* no subpage writes on denali */
 	chip->options |= NAND_NO_SUBPAGE_WRITE;
 
+	/* If "nand-ecc-step-size" DT property is specified, respect it */
+	if (!chip->ecc.size)
+		chip->ecc.size = denali->caps & DENALI_CAPS_ECC_SIZE_1024 ?
+								1024 : 512;
+
 	/*
 	 * Denali Controller only support 15bit and 8bit ECC in MRST,
 	 * so just let controller do 15bit ECC for MLC and 8bit ECC for
 	 * SLC if possible.
 	 * */
 	if (!nand_is_slc(chip) &&
-			(mtd->oobsize > (denali->bbtskipbytes +
-			ECC_15BITS * (mtd->writesize /
-			ECC_SECTOR_SIZE)))) {
+			mtd->oobsize > denali->bbtskipbytes +
+			ECC_15BITS * (mtd->writesize / chip->ecc.size)) {
 		/* if MLC OOB size is large enough, use 15bit ECC*/
 		chip->ecc.strength = 15;
 		chip->ecc.bytes = ECC_15BITS;
 		iowrite32(15, denali->flash_reg + ECC_CORRECTION);
-	} else if (mtd->oobsize < (denali->bbtskipbytes +
-			ECC_8BITS * (mtd->writesize /
-			ECC_SECTOR_SIZE))) {
+	} else if (mtd->oobsize <
+		   denali->bbtskipbytes + ECC_8BITS * (mtd->writesize / chip->ecc.size)) {
 		pr_err("Your NAND chip OOB is not large enough to contain 8bit ECC correction codes");
 		goto failed_req_irq;
 	} else {
@@ -1616,8 +1618,6 @@ int denali_init(struct denali_nand_info *denali)
 
 	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
 
-	/* override the default read operations */
-	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;
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index d621b74..5209625 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -396,8 +396,6 @@
 #define MODE_10    0x08000000
 #define MODE_11    0x0C000000
 
-#define ECC_SECTOR_SIZE     512
-
 struct nand_buf {
 	int head;
 	int tail;
@@ -434,6 +432,7 @@ struct denali_nand_info {
 #define DENALI_CAPS_HW_ECC_FIXUP		BIT(0)
 #define DENALI_CAPS_DMA_64BIT			BIT(1)
 #define DENALI_CAPS_NEW_N_BANKS_FORMAT		BIT(2)
+#define DENALI_CAPS_ECC_SIZE_1024		BIT(3)
 };
 
 extern int denali_init(struct denali_nand_info *denali);
-- 
2.7.4

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

#1534234 — Re: [PATCH 33/39] mtd: nand: denali: support 1024 byte ECC step size

On Sun, Nov 27, 2016 at 03:06:19AM +0900, Masahiro Yamada wrote:
> This driver was originally written for the Intel MRST platform with
> several platform specific parameters hard-coded.  Another thing we
> need to fix is the hard-coded ECC step size.  Currently, it is
> defined as follows:
> 
>   #define ECC_SECTOR_SIZE 512
> 
> (somehow, it is defined in both denali.c and denali.h)
> 
> This must be avoided because the Denali IP supports 1024 byte ECC
> size as well.  Add a new flag DENALI_CAPS_ECC_SIZE_1024. If it is
> specified, ecc.size is set to 1024, otherwise set to 512.
> 
> We can use "nand-ecc-step-size" DT property to override the ecc.size
> if we want, but this capability flag can provide the reasonable
> default because it is associated with the DT compatible strings.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  .../devicetree/bindings/mtd/denali-nand.txt        |  4 ++++

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

>  drivers/mtd/nand/denali.c                          | 26 +++++++++++-----------
>  drivers/mtd/nand/denali.h                          |  3 +--
>  3 files changed, 18 insertions(+), 15 deletions(-)

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

#1530755 — [PATCH 10/39] mtd: nand: denali: remove redundant if conditional of erased_check

This code block is nested by double "if (check_erase_page)".
Remove the redundant inner one.

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

 drivers/mtd/nand/denali.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 753e9a02..ab59371 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1157,12 +1157,10 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 		read_oob_data(mtd, chip->oob_poi, denali->page);
 
 		/* check ECC failures that may have occurred on erased pages */
-		if (check_erased_page) {
-			if (!is_erased(buf, mtd->writesize))
-				mtd->ecc_stats.failed++;
-			if (!is_erased(chip->oob_poi, mtd->oobsize))
-				mtd->ecc_stats.failed++;
-		}
+		if (!is_erased(buf, mtd->writesize))
+			mtd->ecc_stats.failed++;
+		if (!is_erased(chip->oob_poi, mtd->oobsize))
+			mtd->ecc_stats.failed++;
 	}
 	return max_bitflips;
 }
-- 
2.7.4

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

#1530756 — [PATCH 38/39] mtd: nand: denali: remove Toshiba, Hynix specific fixup code

The Denali IP can automatically detect device parameters such as
page size, device width, etc. and this driver currently relies on it.
However, this hardware function is problematic.

[1] Due to a hardware bug, various misdetected cases are known.
    That is why get_toshiba_nand_para(), get_hynix_nand_para() exist
    to fix the misdetected parameters.  It is not realistic to add a
    new NAND device to the *black list* every time we are hit by a
    misdetected case.  We would never be able to guarantee that all
    the cases are covered.

[2] Because this feature is unreliable, it is disabled on some
    platforms.

The nand_scan_ident() sets device parameters such as mtd->writesize,
mtd->erasesize, etc. in a more tested way.  We should not set the
hardware registers in a different, unreliable way.  Instead, set
the parameters from nand_scan_ident() back to the registers.

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

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

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index df174ca..1aa19ec 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -338,35 +338,6 @@ static void get_samsung_nand_para(struct denali_nand_info *denali, u8 device_id)
 	}
 }
 
-static void get_toshiba_nand_para(struct denali_nand_info *denali)
-{
-	/*
-	 * Workaround to fix a controller bug which reports a wrong
-	 * spare area size for some kind of Toshiba NAND device
-	 */
-	if ((ioread32(denali->flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
-		(ioread32(denali->flash_reg + DEVICE_SPARE_AREA_SIZE) == 64))
-		iowrite32(216, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
-}
-
-static void get_hynix_nand_para(struct denali_nand_info *denali, u8 device_id)
-{
-	switch (device_id) {
-	case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
-	case 0xD7: /* Hynix H27UDG8VEM, H27UCG8UDM or H27UCG8V5A */
-		iowrite32(128, denali->flash_reg + PAGES_PER_BLOCK);
-		iowrite32(4096, denali->flash_reg + DEVICE_MAIN_AREA_SIZE);
-		iowrite32(224, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
-		iowrite32(0, denali->flash_reg + DEVICE_WIDTH);
-		break;
-	default:
-		dev_warn(denali->dev,
-			 "Unknown Hynix NAND (Device ID: 0x%x).\n"
-			 "Will use default parameter values instead.\n",
-			 device_id);
-	}
-}
-
 /*
  * determines how many NAND chips are connected to the controller. Note for
  * Intel CE4100 devices we don't support more than one device.
@@ -468,10 +439,6 @@ static u16 denali_nand_timing_set(struct denali_nand_info *denali)
 			return FAIL;
 	} else if (maf_id == 0xEC) { /* Samsung NAND */
 		get_samsung_nand_para(denali, device_id);
-	} else if (maf_id == 0x98) { /* Toshiba NAND */
-		get_toshiba_nand_para(denali);
-	} else if (maf_id == 0xAD) { /* Hynix NAND */
-		get_hynix_nand_para(denali, device_id);
 	}
 
 	dev_info(denali->dev,
@@ -1661,6 +1628,12 @@ int denali_init(struct denali_nand_info *denali)
 						chip->ecc.strength);
 
 	iowrite32(chip->ecc.strength, denali->flash_reg + ECC_CORRECTION);
+	iowrite32(mtd->erasesize / mtd->writesize,
+		  denali->flash_reg + PAGES_PER_BLOCK);
+	iowrite32(denali->nand.options & NAND_BUSWIDTH_16 ? 1 : 0,
+		  denali->flash_reg + DEVICE_WIDTH);
+	iowrite32(mtd->writesize, denali->flash_reg + DEVICE_MAIN_AREA_SIZE);
+	iowrite32(mtd->oobsize, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
 
 	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
 
-- 
2.7.4

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

#1530901 — Re: [PATCH 38/39] mtd: nand: denali: remove Toshiba, Hynix specific fixup code

On Sun, 27 Nov 2016 03:06:24 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> The Denali IP can automatically detect device parameters such as
> page size, device width, etc. and this driver currently relies on it.
> However, this hardware function is problematic.
> 
> [1] Due to a hardware bug, various misdetected cases are known.
>     That is why get_toshiba_nand_para(), get_hynix_nand_para() exist
>     to fix the misdetected parameters.  It is not realistic to add a
>     new NAND device to the *black list* every time we are hit by a
>     misdetected case.  We would never be able to guarantee that all
>     the cases are covered.
> 
> [2] Because this feature is unreliable, it is disabled on some
>     platforms.
> 
> The nand_scan_ident() sets device parameters such as mtd->writesize,
> mtd->erasesize, etc. in a more tested way.  We should not set the
> hardware registers in a different, unreliable way.  Instead, set
> the parameters from nand_scan_ident() back to the registers.
> 

Thanks a lot for fixing that.

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/mtd/nand/denali.c | 39 ++++++---------------------------------
>  1 file changed, 6 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index df174ca..1aa19ec 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -338,35 +338,6 @@ static void get_samsung_nand_para(struct denali_nand_info *denali, u8 device_id)
>  	}
>  }
>  
> -static void get_toshiba_nand_para(struct denali_nand_info *denali)
> -{
> -	/*
> -	 * Workaround to fix a controller bug which reports a wrong
> -	 * spare area size for some kind of Toshiba NAND device
> -	 */
> -	if ((ioread32(denali->flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
> -		(ioread32(denali->flash_reg + DEVICE_SPARE_AREA_SIZE) == 64))
> -		iowrite32(216, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
> -}
> -
> -static void get_hynix_nand_para(struct denali_nand_info *denali, u8 device_id)
> -{
> -	switch (device_id) {
> -	case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
> -	case 0xD7: /* Hynix H27UDG8VEM, H27UCG8UDM or H27UCG8V5A */
> -		iowrite32(128, denali->flash_reg + PAGES_PER_BLOCK);
> -		iowrite32(4096, denali->flash_reg + DEVICE_MAIN_AREA_SIZE);
> -		iowrite32(224, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
> -		iowrite32(0, denali->flash_reg + DEVICE_WIDTH);
> -		break;
> -	default:
> -		dev_warn(denali->dev,
> -			 "Unknown Hynix NAND (Device ID: 0x%x).\n"
> -			 "Will use default parameter values instead.\n",
> -			 device_id);
> -	}
> -}
> -
>  /*
>   * determines how many NAND chips are connected to the controller. Note for
>   * Intel CE4100 devices we don't support more than one device.
> @@ -468,10 +439,6 @@ static u16 denali_nand_timing_set(struct denali_nand_info *denali)
>  			return FAIL;
>  	} else if (maf_id == 0xEC) { /* Samsung NAND */
>  		get_samsung_nand_para(denali, device_id);
> -	} else if (maf_id == 0x98) { /* Toshiba NAND */
> -		get_toshiba_nand_para(denali);
> -	} else if (maf_id == 0xAD) { /* Hynix NAND */
> -		get_hynix_nand_para(denali, device_id);
>  	}
>  
>  	dev_info(denali->dev,
> @@ -1661,6 +1628,12 @@ int denali_init(struct denali_nand_info *denali)
>  						chip->ecc.strength);
>  
>  	iowrite32(chip->ecc.strength, denali->flash_reg + ECC_CORRECTION);
> +	iowrite32(mtd->erasesize / mtd->writesize,
> +		  denali->flash_reg + PAGES_PER_BLOCK);
> +	iowrite32(denali->nand.options & NAND_BUSWIDTH_16 ? 1 : 0,
> +		  denali->flash_reg + DEVICE_WIDTH);
> +	iowrite32(mtd->writesize, denali->flash_reg + DEVICE_MAIN_AREA_SIZE);
> +	iowrite32(mtd->oobsize, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
>  
>  	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
>  

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

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

+Andy

Hi Masahiro,

On Sun, 27 Nov 2016 03:05:46 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

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

Thanks for posting this 2nd round of patches, I know have a clearer
view of what you're trying to achieve.
Could you be a bit more specific about the remaining rework (your 3rd
round)?

Also, if you don't mind, I'd like to have reviews and testing from intel
users before applying the series. Can you Cc Andy (and possibly other
intel maintainers) for the next round.

Thanks,

Boris

> 
> 
> 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(-)
> 

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

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

Hi.

2016-11-28 0:04 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> +Andy
>
> Hi Masahiro,
>
> On Sun, 27 Nov 2016 03:05:46 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
>> 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.
>>
>
> Thanks for posting this 2nd round of patches, I know have a clearer
> view of what you're trying to achieve.
> Could you be a bit more specific about the remaining rework (your 3rd
> round)?


[1]
I want to remove
get_samsung_nand_para()
get_onfi_nand_para()

The driver should not hard-code timing parameters of Samsung specific
chips.  For ONFI, it is duplicating effort of the core framework.

I am thinking if it would be possible to implement
chip->setup_data_interface() in order to set up
timings in a generic way.

[2]
Remove driver-internal bounce buffer.
The current Denali driver allocate DMA_BIDIRECTIONAL buffer
to use it as a driver-internal bounce buffer.

The hardware transfer page data into the bounce buffer,
then CPU copies from the bounce buffer to a given buf (and oob_poi).
This is not efficient.

So, I want to set NAND_USE_BOUNCE_BUFFER flag
and do dma_map_single directly for a given buffer.

[3]
Fix raw and oob callbacks.

I asked in another thread,
the current driver just puts the physically accessed OOB data
into oob_poi, which is not a collection of ECC data.
Raw write/read() are wrong as well.

After fixing those, enable BBT scan by removing the following flag:
    /* skip the scan for now until we have OOB read and write support */
    chip->options |= NAND_SKIP_BBTSCAN;



> Also, if you don't mind, I'd like to have reviews and testing from intel
> users before applying the series. Can you Cc Andy (and possibly other
> intel maintainers) for the next round.

Sure.

Anyway, this series already missed the pull-req for 4.10-rc1,
we have plenty of time until 4.11-rc1.

Review/test from Intel engineers are very appreciated
because I have no access to their boards.


-- 
Best Regards
Masahiro Yamada

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

Page 3 of 4 — ← Prev page 1 2 [3] 4  Next page →

Back to top | Article view | linux.kernel

csiph-web