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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK7LNASJLbizHEh8mUUVDvTKiRMCSvyYtdrQzwN5tHmCV8iYOA@mail.gmail.com>
Date:   Fri, 2 Dec 2016 11:54:17 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Rob Herring <robh@...nel.org>
Cc:     linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        Marek Vasut <marek.vasut@...il.com>,
        Brian Norris <computersforpeace@...il.com>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>,
        Cyrille Pitchen <cyrille.pitchen@...el.com>,
        Mark Rutland <mark.rutland@....com>,
        Dinh Nguyen <dinguyen@...era.com>
Subject: Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for
 UniPhier SoC variants

Hi Rob,
(+CC Dinh)

2016-12-02 1:05 GMT+09:00 Rob Herring <robh@...nel.org>:
> On Sun, Nov 27, 2016 at 03:06:25AM +0900, Masahiro Yamada wrote:
>> Add two compatible strings for UniPhier SoCs.  The revision register
>> on both shows revision 5.0, but they are different hardware.
>>
>> Features:
>>  - DMA engine with 64 bit physical address support
>>  - 1024 byte ECC step size
>>  - 8 / 16 / 24 bit ECC strength
>>  - The n_banks format depends on SoC
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@...ionext.com>
>> ---
>>
>>  .../devicetree/bindings/mtd/denali-nand.txt        | 10 +++++--
>>  drivers/mtd/nand/denali_dt.c                       | 33 ++++++++++++++++++++--
>>  2 files changed, 38 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
>> index 51fe195..cea46e2 100644
>> --- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
>> @@ -1,13 +1,19 @@
>>  * Denali NAND controller
>>
>>  Required properties:
>> -  - compatible : should be "denali,denali-nand-dt"
>> +  - compatible : should be one of the following:
>> +      "denali,denali-nand-dt"
>
> There are multiple things wrong with this string. denali,denali is
> redundant is one.

One more redundancy; "-dt" is weird because
DT compatible should be a name of hardware.


> It's also fairly useless as this IP has several
> versions and numerous configuration options IIRC. This should be
> deprecated IMO.

Right.  There are several customizable parameters for this IP,
so a generic compatible string like this is probably useless.

This DT binding was added by commit 30f9f2f for Altera SOCFPGA,
A funny thing is that they upstreamed DT binding, but they did not upstream
needed changes for the Denali driver core.
So, the mainline driver has never worked on SOCFPGA
(or any of DT-based SoCs).



>> +      "denali,denali-nand-uniphier-v5a"
>> +      "denali,denali-nand-uniphier-v5b"
>
> Use your vendor prefix, not denali. The 2nd denali can probably be
> dropped because it is not likely you have another kind of nand
> controller in the SoC.

Hmm, your statement implies that a vendor prefix
belongs to an SoC vendor, not an IP vendor.
(I was not quite sure about this.)


It is unlikely to happen to have two different NAND controllers on one SoC.
But, we used a different NAND controller for our SoC family before
introducing the Denali IP.
It also implies that Socionext may use a different NAND IP in the future.
I'd like to include "denali" somewhere because it is clearly associated with
the driver name.
Also, this will give an idea what kind of _basic_ hardware is used,
even though we know various parameters are customizable.



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


I think Plan B is nearer to your suggestion,
and I think it is OK for Socionext (hopefully for Altera too).





-- 
Best Regards
Masahiro Yamada