From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752547AbeBSJ5e (ORCPT ); Mon, 19 Feb 2018 04:57:34 -0500 Received: from mail-qk0-f194.google.com ([209.85.220.194]:47021 "EHLO mail-qk0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752129AbeBSJ5c (ORCPT ); Mon, 19 Feb 2018 04:57:32 -0500 X-Google-Smtp-Source: AH8x226dfW8bn8Sted7H6cy31n9JczFnUOAbBUZdbZvD7l4V1tgZlUpMTtugyiOsQRinoWC22Uh6Lc1YXyEcocWTF20= MIME-Version: 1.0 In-Reply-To: <20180213101412.5717-3-liwei213@huawei.com> References: <20180213101412.5717-1-liwei213@huawei.com> <20180213101412.5717-3-liwei213@huawei.com> From: Arnd Bergmann Date: Mon, 19 Feb 2018 10:57:30 +0100 X-Google-Sender-Auth: RNNxwvkRaL32D878LitCzPWH5eU Message-ID: Subject: Re: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs To: Li Wei Cc: Rob Herring , Mark Rutland , Wei Xu , Catalin Marinas , Will Deacon , Vinayak Holikatti , "James E.J. Bottomley" , "Martin K. Petersen" , Kevin Hilman , Gregory CLEMENT , Thomas Petazzoni , Masahiro Yamada , Riku Voipio , Thierry Reding , Krzysztof Kozlowski , Eric Anholt , DTML , Linux Kernel Mailing List , Linux ARM , linux-scsi , zangleigang@hisilicon.com, gengjianfeng@hisilicon.com, Guodong Xu , Zhangfei Gao , "Fengbaopeng (kevin, Kirin Solution Dept)" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 13, 2018 at 11:14 AM, Li Wei wrote: > add ufs node document for Hisilicon. > > Signed-off-by: Li Wei > --- > Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 37 ++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt I'm pretty sure we've discussed it before, but can you make this so that the generic properties are part of the ufshcd binding, and you refer to it from here, only describing in what ways the hisi ufs binding differs from the standard? > diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt > new file mode 100644 > index 000000000000..0d21b57496cf > --- /dev/null > +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt > @@ -0,0 +1,37 @@ > +* Hisilicon Universal Flash Storage (UFS) Host Controller > + > +UFS nodes are defined to describe on-chip UFS hardware macro. > +Each UFS Host Controller should have its own node. > + > +Required properties: > +- compatible : compatible list, contains one of the following - > + "hisilicon,hi3660-ufs", "jedec,ufs-1.1" for hisi ufs > + host controller present on Hi36xx chipset. > +- reg : should contain UFS register address space & UFS SYS CTRL register address, > +- interrupt-parent : interrupt device > +- interrupts : interrupt number > +- clocks : List of phandle and clock specifier pairs > +- clock-names : List of clock input name strings sorted in the same > + order as the clocks property. "ref_clk", "phy_clk" is optional The clock names sound generic enough, should we have both in the generic binding? > +- resets : reset node register, one reset the clk and the other reset the controller > +- reset-names : describe reset node register This looks incomplete. What is the name of the reset line supposed to be? I'd also suggest you document it in the ufshcd binding instead. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs Date: Mon, 19 Feb 2018 10:57:30 +0100 Message-ID: References: <20180213101412.5717-1-liwei213@huawei.com> <20180213101412.5717-3-liwei213@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180213101412.5717-3-liwei213@huawei.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Li Wei Cc: Mark Rutland , Catalin Marinas , Will Deacon , Eric Anholt , Krzysztof Kozlowski , Vinayak Holikatti , "James E.J. Bottomley" , linux-scsi , Kevin Hilman , zangleigang@hisilicon.com, Wei Xu , Zhangfei Gao , Thierry Reding , DTML , "Fengbaopeng (kevin, Kirin Solution Dept)" , Rob Herring , Gregory CLEMENT , Linux ARM , Thomas Petazzoni , Guodong Xu , "Martin K. Petersen" List-Id: devicetree@vger.kernel.org On Tue, Feb 13, 2018 at 11:14 AM, Li Wei wrote: > add ufs node document for Hisilicon. > > Signed-off-by: Li Wei > --- > Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 37 ++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt I'm pretty sure we've discussed it before, but can you make this so that the generic properties are part of the ufshcd binding, and you refer to it from here, only describing in what ways the hisi ufs binding differs from the standard? > diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt > new file mode 100644 > index 000000000000..0d21b57496cf > --- /dev/null > +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt > @@ -0,0 +1,37 @@ > +* Hisilicon Universal Flash Storage (UFS) Host Controller > + > +UFS nodes are defined to describe on-chip UFS hardware macro. > +Each UFS Host Controller should have its own node. > + > +Required properties: > +- compatible : compatible list, contains one of the following - > + "hisilicon,hi3660-ufs", "jedec,ufs-1.1" for hisi ufs > + host controller present on Hi36xx chipset. > +- reg : should contain UFS register address space & UFS SYS CTRL register address, > +- interrupt-parent : interrupt device > +- interrupts : interrupt number > +- clocks : List of phandle and clock specifier pairs > +- clock-names : List of clock input name strings sorted in the same > + order as the clocks property. "ref_clk", "phy_clk" is optional The clock names sound generic enough, should we have both in the generic binding? > +- resets : reset node register, one reset the clk and the other reset the controller > +- reset-names : describe reset node register This looks incomplete. What is the name of the reset line supposed to be? I'd also suggest you document it in the ufshcd binding instead. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Mon, 19 Feb 2018 10:57:30 +0100 Subject: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs In-Reply-To: <20180213101412.5717-3-liwei213@huawei.com> References: <20180213101412.5717-1-liwei213@huawei.com> <20180213101412.5717-3-liwei213@huawei.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Feb 13, 2018 at 11:14 AM, Li Wei wrote: > add ufs node document for Hisilicon. > > Signed-off-by: Li Wei > --- > Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 37 ++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt I'm pretty sure we've discussed it before, but can you make this so that the generic properties are part of the ufshcd binding, and you refer to it from here, only describing in what ways the hisi ufs binding differs from the standard? > diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt > new file mode 100644 > index 000000000000..0d21b57496cf > --- /dev/null > +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt > @@ -0,0 +1,37 @@ > +* Hisilicon Universal Flash Storage (UFS) Host Controller > + > +UFS nodes are defined to describe on-chip UFS hardware macro. > +Each UFS Host Controller should have its own node. > + > +Required properties: > +- compatible : compatible list, contains one of the following - > + "hisilicon,hi3660-ufs", "jedec,ufs-1.1" for hisi ufs > + host controller present on Hi36xx chipset. > +- reg : should contain UFS register address space & UFS SYS CTRL register address, > +- interrupt-parent : interrupt device > +- interrupts : interrupt number > +- clocks : List of phandle and clock specifier pairs > +- clock-names : List of clock input name strings sorted in the same > + order as the clocks property. "ref_clk", "phy_clk" is optional The clock names sound generic enough, should we have both in the generic binding? > +- resets : reset node register, one reset the clk and the other reset the controller > +- reset-names : describe reset node register This looks incomplete. What is the name of the reset line supposed to be? I'd also suggest you document it in the ufshcd binding instead. Arnd