On Thu, Dec 1, 2016 at 8:54 PM, Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote: > Hi Rob, > (+CC Dinh) > > 2016-12-02 1:05 GMT+09:00 Rob Herring <robh@xxxxxxxxxx>: >> 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@xxxxxxxxxxxxx> >>> --- >>> >>> .../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.) Right. We do have some IP vendor strings, but they are usually fallbacks. Sometimes, I guess they are useful, but IMO over time they prove to not be useful. > 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) 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. I simply would do "socionext,uniphier-v5b-nand" (and v5a). The fact that it is denali is part of the documentation. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
