答复: 答复: [PATCH v9 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

2018-05-16 Thread liwei (CM)
Hi, Rob

-邮件原件-
发件人: Rob Herring [mailto:r...@kernel.org] 
发送时间: 2018年5月16日 21:16
收件人: liwei (CM)
抄送: mark.rutl...@arm.com; catalin.mari...@arm.com; will.dea...@arm.com; 
vinholika...@gmail.com; j...@linux.vnet.ibm.com; martin.peter...@oracle.com; 
khil...@baylibre.com; a...@arndb.de; gregory.clem...@free-electrons.com; 
thomas.petazz...@free-electrons.com; yamada.masah...@socionext.com; 
riku.voi...@linaro.org; tred...@nvidia.com; k...@kernel.org; 
devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; 
linux-arm-ker...@lists.infradead.org; linux-scsi@vger.kernel.org; zangleigang; 
Gengjianfeng; guodong...@linaro.org
主题: Re: 答复: [PATCH v9 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

On Tue, Apr 24, 2018 at 8:54 AM, liwei (CM)  wrote:
> Hi, Rob
>
> Thanks for your patience.
>
> Hi, Arnd
>
> From Rob's suggestion, we have to list the properties node in ufs-hisi.txt 
> bingings even if documented in the common binding.
>
> -邮件原件-
> 发件人: Rob Herring [mailto:r...@kernel.org]
> 发送时间: 2018年4月24日 20:58
> 收件人: liwei (CM)
> 抄送: mark.rutl...@arm.com; catalin.mari...@arm.com; will.dea...@arm.com; 
> vinholika...@gmail.com; j...@linux.vnet.ibm.com; martin.peter...@oracle.com; 
> khil...@baylibre.com; a...@arndb.de; gregory.clem...@free-electrons.com; 
> thomas.petazz...@free-electrons.com; yamada.masah...@socionext.com; 
> riku.voi...@linaro.org; tred...@nvidia.com; k...@kernel.org; 
> devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; 
> linux-arm-ker...@lists.infradead.org; linux-scsi@vger.kernel.org; 
> zangleigang; Gengjianfeng; guodong...@linaro.org
> 主题: Re: [PATCH v9 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
>
> On Tue, Apr 17, 2018 at 10:08:11PM +0800, Li Wei wrote:
>> add ufs node document for Hisilicon.
>>
>> Signed-off-by: Li Wei 
>> ---
>>  Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 29 
>> ++
>>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt  | 10 +---
>>  2 files changed, 36 insertions(+), 3 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt 
>> b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> new file mode 100644
>> index ..d49ab7d8f31d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> @@ -0,0 +1,29 @@
>> +* 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
>> +- resets: reset node register, the "arst" corresponds to reset 
>> the APB/AXI bus.
>
> arst belongs in reset-names.
>
> OK, I will fix it in next patch;
>
>> +- reset-names   : describe reset node register
>
> What happened to clocks? You still have to list which ones apply even if
> documented in the common binding.
>
> OK, I will fix it in next patch;
>
>> +
>> +Example:
>> +
>> + ufs: ufs@ff3b {
>> + compatible = "hisilicon,hi3660-ufs", "jedec,ufs-1.1";
>> + /* 0: HCI standard */
>> + /* 1: UFS SYS CTRL */
>> + reg = <0x0 0xff3b 0x0 0x1000>,
>> + <0x0 0xff3b1000 0x0 0x1000>;
>> + interrupt-parent = <&gic>;
>> + interrupts = ;
>> + /* offset: 0x84; bit: 7  */
>> + resets = <&crg_rst 0x84 7>;
>> + reset-names = "arst";
>> + };
>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt 
>> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> index c39dfef76a18..adcfb79f63f5 100644
>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> @@ -41,6 +41,8 @@ Optional properties:
>>  -lanes-per-direction : number of lanes available per direction - either 1 
>> or 2.
>> Note that it 

Re: 答复: [PATCH v9 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

2018-05-16 Thread Rob Herring
On Tue, Apr 24, 2018 at 8:54 AM, liwei (CM)  wrote:
> Hi, Rob
>
> Thanks for your patience.
>
> Hi, Arnd
>
> From Rob's suggestion, we have to list the properties node in ufs-hisi.txt 
> bingings even if documented in the common binding.
>
> -邮件原件-
> 发件人: Rob Herring [mailto:r...@kernel.org]
> 发送时间: 2018年4月24日 20:58
> 收件人: liwei (CM)
> 抄送: mark.rutl...@arm.com; catalin.mari...@arm.com; will.dea...@arm.com; 
> vinholika...@gmail.com; j...@linux.vnet.ibm.com; martin.peter...@oracle.com; 
> khil...@baylibre.com; a...@arndb.de; gregory.clem...@free-electrons.com; 
> thomas.petazz...@free-electrons.com; yamada.masah...@socionext.com; 
> riku.voi...@linaro.org; tred...@nvidia.com; k...@kernel.org; 
> devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; 
> linux-arm-ker...@lists.infradead.org; linux-scsi@vger.kernel.org; 
> zangleigang; Gengjianfeng; guodong...@linaro.org
> 主题: Re: [PATCH v9 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
>
> On Tue, Apr 17, 2018 at 10:08:11PM +0800, Li Wei wrote:
>> add ufs node document for Hisilicon.
>>
>> Signed-off-by: Li Wei 
>> ---
>>  Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 29 
>> ++
>>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt  | 10 +---
>>  2 files changed, 36 insertions(+), 3 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt 
>> b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> new file mode 100644
>> index ..d49ab7d8f31d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> @@ -0,0 +1,29 @@
>> +* 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
>> +- resets: reset node register, the "arst" corresponds to reset 
>> the APB/AXI bus.
>
> arst belongs in reset-names.
>
> OK, I will fix it in next patch;
>
>> +- reset-names   : describe reset node register
>
> What happened to clocks? You still have to list which ones apply even if
> documented in the common binding.
>
> OK, I will fix it in next patch;
>
>> +
>> +Example:
>> +
>> + ufs: ufs@ff3b {
>> + compatible = "hisilicon,hi3660-ufs", "jedec,ufs-1.1";
>> + /* 0: HCI standard */
>> + /* 1: UFS SYS CTRL */
>> + reg = <0x0 0xff3b 0x0 0x1000>,
>> + <0x0 0xff3b1000 0x0 0x1000>;
>> + interrupt-parent = <&gic>;
>> + interrupts = ;
>> + /* offset: 0x84; bit: 7  */
>> + resets = <&crg_rst 0x84 7>;
>> + reset-names = "arst";
>> + };
>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt 
>> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> index c39dfef76a18..adcfb79f63f5 100644
>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> @@ -41,6 +41,8 @@ Optional properties:
>>  -lanes-per-direction : number of lanes available per direction - either 1 
>> or 2.
>> Note that it is assume same number of lanes is used 
>> both
>> directions at once. If not specified, default is 2 
>> lanes per direction.
>> +- resets: reset node register, the "rst" corresponds to reset 
>> the whole UFS IP.
>> +- reset-names   : describe reset node register
>
> Does your controller have 1 or 2 resets? There's no point in adding this
> here if it doesn't apply to your controller.
>
> There are 2 reset in our soc init, the "rst" corresponds to reset the whole 
> UFS IP, and " arst " only reset the APB/AXI bus.
> Discussed with our soc colleagues that "arst" is assert by default and needs 
> to deassert,but it done in bootloader,so will remove 'arst' in next patch.
>
> About the 'reset' property,it seems that Arnd Bergmann has different 
> suggestion,he suggested that add 'rst' to ufshcd-pltfrm because it seems 
> common.
> But it looks like only our soc init needs it. What's your opinion? Does it 
> still needs add to common bindings?

For a single reset, then I'm fine with it being in the common
bindings. If there are multiple then it should be in the specific
bindings as the reset names are likely different.

Rob


Re: [PATCH v9 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

2018-04-24 Thread Rob Herring
On Tue, Apr 17, 2018 at 10:08:11PM +0800, Li Wei wrote:
> add ufs node document for Hisilicon.
> 
> Signed-off-by: Li Wei 
> ---
>  Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 29 
> ++
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt  | 10 +---
>  2 files changed, 36 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt 
> b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
> new file mode 100644
> index ..d49ab7d8f31d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
> @@ -0,0 +1,29 @@
> +* 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
> +- resets: reset node register, the "arst" corresponds to reset 
> the APB/AXI bus.

arst belongs in reset-names.

> +- reset-names   : describe reset node register

What happened to clocks? You still have to list which ones apply even if 
documented in the common binding.

> +
> +Example:
> +
> + ufs: ufs@ff3b {
> + compatible = "hisilicon,hi3660-ufs", "jedec,ufs-1.1";
> + /* 0: HCI standard */
> + /* 1: UFS SYS CTRL */
> + reg = <0x0 0xff3b 0x0 0x1000>,
> + <0x0 0xff3b1000 0x0 0x1000>;
> + interrupt-parent = <&gic>;
> + interrupts = ;
> + /* offset: 0x84; bit: 7  */
> + resets = <&crg_rst 0x84 7>;
> + reset-names = "arst";
> + };
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt 
> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef76a18..adcfb79f63f5 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -41,6 +41,8 @@ Optional properties:
>  -lanes-per-direction : number of lanes available per direction - either 1 or 
> 2.
> Note that it is assume same number of lanes is used 
> both
> directions at once. If not specified, default is 2 
> lanes per direction.
> +- resets: reset node register, the "rst" corresponds to reset 
> the whole UFS IP.
> +- reset-names   : describe reset node register

Does your controller have 1 or 2 resets? There's no point in adding this 
here if it doesn't apply to your controller.


>  Note: If above properties are not defined it can be assumed that the supply
>  regulators or clocks are always on.
> @@ -61,9 +63,11 @@ Example:
>   vccq-max-microamp = 20;
>   vccq2-max-microamp = 20;
>  
> - clocks = <&core 0>, <&ref 0>, <&iface 0>;
> - clock-names = "core_clk", "ref_clk", "iface_clk";
> - freq-table-hz = <1 2>, <0 0>, <0 0>;
> + clocks = <&core 0>, <&ref 0>, <&phy 0>, <&iface 0>;
> + clock-names = "core_clk", "ref_clk", "phy_clk", "iface_clk";
> + freq-table-hz = <1 2>, <0 0>, <0 0>, <0 0>;
> + resets = <&reset 0 1>;
> + reset-names = "rst";
>   phys = <&ufsphy1>;
>   phy-names = "ufsphy";
>   };
> -- 
> 2.15.0
> 


[PATCH v9 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

2018-04-17 Thread Li Wei
add ufs node document for Hisilicon.

Signed-off-by: Li Wei 
---
 Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 29 ++
 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt  | 10 +---
 2 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt

diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt 
b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
new file mode 100644
index ..d49ab7d8f31d
--- /dev/null
+++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
@@ -0,0 +1,29 @@
+* 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
+- resets: reset node register, the "arst" corresponds to reset the 
APB/AXI bus.
+- reset-names   : describe reset node register
+
+Example:
+
+   ufs: ufs@ff3b {
+   compatible = "hisilicon,hi3660-ufs", "jedec,ufs-1.1";
+   /* 0: HCI standard */
+   /* 1: UFS SYS CTRL */
+   reg = <0x0 0xff3b 0x0 0x1000>,
+   <0x0 0xff3b1000 0x0 0x1000>;
+   interrupt-parent = <&gic>;
+   interrupts = ;
+   /* offset: 0x84; bit: 7  */
+   resets = <&crg_rst 0x84 7>;
+   reset-names = "arst";
+   };
diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt 
b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index c39dfef76a18..adcfb79f63f5 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -41,6 +41,8 @@ Optional properties:
 -lanes-per-direction   : number of lanes available per direction - either 1 or 
2.
  Note that it is assume same number of lanes is used 
both
  directions at once. If not specified, default is 2 
lanes per direction.
+- resets: reset node register, the "rst" corresponds to reset the 
whole UFS IP.
+- reset-names   : describe reset node register
 
 Note: If above properties are not defined it can be assumed that the supply
 regulators or clocks are always on.
@@ -61,9 +63,11 @@ Example:
vccq-max-microamp = 20;
vccq2-max-microamp = 20;
 
-   clocks = <&core 0>, <&ref 0>, <&iface 0>;
-   clock-names = "core_clk", "ref_clk", "iface_clk";
-   freq-table-hz = <1 2>, <0 0>, <0 0>;
+   clocks = <&core 0>, <&ref 0>, <&phy 0>, <&iface 0>;
+   clock-names = "core_clk", "ref_clk", "phy_clk", "iface_clk";
+   freq-table-hz = <1 2>, <0 0>, <0 0>, <0 0>;
+   resets = <&reset 0 1>;
+   reset-names = "rst";
phys = <&ufsphy1>;
phy-names = "ufsphy";
};
-- 
2.15.0