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

From: Dinh Nguyen
Date: Sat Dec 03 2016 - 17:09:20 EST


Hi,

On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut <marek.vasut@xxxxxxxxx> 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@xxxxxxxxxx>:
>>
>>>>
>>>>
>>>> (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@xxxxxxxxxx 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.

Dinh