Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752581AbcLBQ1l (ORCPT ); Fri, 2 Dec 2016 11:27:41 -0500 Received: from mail.kernel.org ([198.145.29.136]:46350 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750750AbcLBQ1j (ORCPT ); Fri, 2 Dec 2016 11:27:39 -0500 MIME-Version: 1.0 In-Reply-To: References: <1480183585-592-1-git-send-email-yamada.masahiro@socionext.com> <1480183585-592-40-git-send-email-yamada.masahiro@socionext.com> <20161201160511.ahlibszokg547wxk@rob-hp-laptop> From: Rob Herring Date: Fri, 2 Dec 2016 10:26:30 -0600 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants To: Masahiro Yamada Cc: "linux-mtd@lists.infradead.org" , "devicetree@vger.kernel.org" , Linux Kernel Mailing List , Boris Brezillon , Marek Vasut , Brian Norris , Richard Weinberger , David Woodhouse , Cyrille Pitchen , Mark Rutland , Dinh Nguyen Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3890 Lines: 99 On Thu, Dec 1, 2016 at 8:54 PM, Masahiro Yamada wrote: > Hi Rob, > (+CC Dinh) > > 2016-12-02 1:05 GMT+09:00 Rob Herring : >> 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 >>> --- >>> >>> .../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