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

[liwei (CM)]

Hi, Arnd

Thanks for your patiences.

-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2018年3月28日 20:50
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); 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; Gengjianfeng; Guodong Xu; Zhangfei Gao; Fengbaopeng 
(kevin, Kirin Solution Dept); Yaniv Gardi
主题: Re: 答复: 答复: 答复: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for 
hisi-ufs

On Tue, Mar 27, 2018 at 8:15 AM, liwei (CM) <liwei...@huawei.com> wrote:
> Hi, Arnd
>
> At present our ufs module mainly has four clocks from the outside:
> hclk_ufs: main clock of ufs controller ,freq is 207.5MHz
> cfg_phy_clk:  configuration clock of MPHY, freq is 51.875MHz
> ref_phy_clk:  reference clock of MPHY from PMU, freq is 19.2MHz
> ref_io_clk:reference clock for the external interface to the device, freq 
> is 19.2MHz
>
> We control two clocks "ref_io_clk" and "cfg_phy_clk" in the driver 
> because the other two are controlled by main clock module and pmu.

I'm not completely sure what you mean with "control" here. Do you mean setting 
the rate and disabling them during runtime power management? What does it mean 
for the clock to be controlled by teh "main clock module and pmu"?

In the driver we only disable/enable "ref_io_clk" and "cfg_phy_clk" during 
runtime power management.

> for this patch, cfg_phy_clk corresponds to "phy_clk", ref_io_clk corresponds 
> to "ref_clk".

I'm not sure I understand the difference between ref_phy_clk and ref_io_clk, 
but it sounds like we should give both of those names in the ufs-platform 
binding.

Your hclk_ufs would appear to correspond to what qualcomm calls core_clk, so 
maybe use that name as well.

cfg_phy_clk seems to be something that qcom would not have, but it's also 
generic enough to list it in the common binding.

Ok, let's add a describe for phy_clk in the common binding.

> So the clks in the patch you give appear to be unsuitable for describing this 
> .And the following clks of qcom are internal clock?
> We didn't describe or pay attention to the clock inside the ufs module.
>
> PHY to controller symbol synchronization clocks:
> "rx_lane0_sync_clk" - RX Lane 0
> "rx_lane1_sync_clk" - RX Lane 1
> "tx_lane0_sync_clk" - TX Lane 0
> "tx_lane1_sync_clk" - TX Lane 1

Right, let's leave those for the qcom private binding.

  Arnd

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

[Arnd Bergmann]

On Tue, Mar 27, 2018 at 8:15 AM, liwei (CM)  wrote:
> Hi, Arnd
>
> At present our ufs module mainly has four clocks from the outside:
> hclk_ufs: main clock of ufs controller ,freq is 207.5MHz
> cfg_phy_clk:  configuration clock of MPHY, freq is 51.875MHz
> ref_phy_clk:  reference clock of MPHY from PMU, freq is 19.2MHz
> ref_io_clk:reference clock for the external interface to the device, freq 
> is 19.2MHz
>
> We control two clocks "ref_io_clk" and "cfg_phy_clk" in the driver because 
> the other
> two are controlled by main clock module and pmu.

I'm not completely sure what you mean with "control" here. Do you mean
setting the
rate and disabling them during runtime power management? What does it mean for
the clock to be controlled by teh "main clock module and pmu"?

> for this patch, cfg_phy_clk corresponds to "phy_clk", ref_io_clk corresponds 
> to "ref_clk".

I'm not sure I understand the difference between ref_phy_clk and
ref_io_clk, but it sounds
like we should give both of those names in the ufs-platform binding.

Your hclk_ufs would appear to correspond to what qualcomm calls core_clk, so
maybe use that name as well.

cfg_phy_clk seems to be something that qcom would not have, but it's
also generic
enough to list it in the common binding.

> So the clks in the patch you give appear to be unsuitable for describing this 
> .And the following clks of qcom are internal clock?
> We didn't describe or pay attention to the clock inside the ufs module.
>
> PHY to controller symbol synchronization clocks:
> "rx_lane0_sync_clk" - RX Lane 0
> "rx_lane1_sync_clk" - RX Lane 1
> "tx_lane0_sync_clk" - TX Lane 0
> "tx_lane1_sync_clk" - TX Lane 1

Right, let's leave those for the qcom private binding.

  Arnd

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

[liwei (CM)]

Hi, Arnd

At present our ufs module mainly has four clocks from the outside:
hclk_ufs: main clock of ufs controller ,freq is 207.5MHz 
cfg_phy_clk:  configuration clock of MPHY, freq is 51.875MHz
ref_phy_clk:  reference clock of MPHY from PMU, freq is 19.2MHz
ref_io_clk:reference clock for the external interface to the device, freq 
is 19.2MHz

We control two clocks "ref_io_clk" and "cfg_phy_clk" in the driver because the 
other two are controlled by main clock module and pmu. 
for this patch, cfg_phy_clk corresponds to "phy_clk", ref_io_clk corresponds to 
"ref_clk".

So the clks in the patch you give appear to be unsuitable for describing this 
.And the following clks of qcom are internal clock? 
We didn't describe or pay attention to the clock inside the ufs module.

PHY to controller symbol synchronization clocks:
"rx_lane0_sync_clk" - RX Lane 0
"rx_lane1_sync_clk" - RX Lane 1
"tx_lane0_sync_clk" - TX Lane 0
"tx_lane1_sync_clk" - TX Lane 1


-邮件原件-
发件人: liwei (CM) 
发送时间: 2018年3月26日 20:02
收件人: 'Arnd Bergmann'
抄送: Rob Herring; Mark Rutland; xuwei (O); 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; Gengjianfeng; Guodong Xu; Zhangfei Gao; Fengbaopeng 
(kevin, Kirin Solution Dept); Yaniv Gardi
主题: 答复: 答复: 答复: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

Hi, Arnd

I'll ask our soc colleagues for help and give a detailed and accurate 
explanation aosp.

Thanks!


-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2018年3月26日 18:42
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); 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; Gengjianfeng; Guodong Xu; Zhangfei Gao; Fengbaopeng 
(kevin, Kirin Solution Dept); Yaniv Gardi
主题: Re: 答复: 答复: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

On Mon, Mar 26, 2018 at 12:26 PM, liwei (CM) <liwei...@huawei.com> wrote:
> 发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd 
> Bergmann
> > 主题: Re: 答复: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for 
> > hisi-ufs On Fri, Mar 23, 2018 at 3:22 AM, liwei (CM) <liwei...@huawei.com> 
> > wrote:
> >> The clock names sound generic enough, should we have both in the generic 
> >> binding?
> >>
> >> Do you mean that add a "phy_clk" to ufshcd-pltfrm 's bindings?
> >> At present, it seems that in the implementation of generic code, 
> >> apart from "ref_clk" may have special processing, other clk will 
> >> not have special processing and simply parse and enable; Referring 
> >> to ufs-qcom binding, I think "phy_clk" can be named "iface_clk", 
> >> this "iface_clk" exists in ufshcd-pltfrm bindings;If so, "ref_clk", 
> >> "iface_clk" are both in the generic binding,we will remove them here. Is 
> >> that okay?
>
> > I'm looking at the generic binding again, and it seems we never 
> > quite managed to fix some minor problems with it. See below for a possible 
> > way to clarify it.
>
> phy_clk is actually given to the phy. But as previously mentioned , we 
> do not have a separate phy to configure ; The clks in the patch you 
> give appear to be unsuitable for describing this .
> Here we can't describe phy_clk in the node "ufsphy1: ufsphy@fc597000" like 
> qcom.
> So can we put it here in our own binding like this?

I think the concept of having a phy clk is generic enough that it's better to 
have that in the common part, others will surely have the same thing, and in 
this case, qcom would be the exception that does not use one.

There are apparently a couple of things related to the phy that may or may not 
require a clk:

- ref_clk: The reference clock on the mipi bus, this is what qcom have, this 
would
  be the 19.2 MHz clock signal.
- one clock to drive the logic block for the PHY itself, if it is included 
within
  the same logical portion of an SoC as the ufshcd, but uses a separate clock.
- Looking at the Android kernel as distributed by google/qualcomm, they have
  four separate clocks described as

PHY to controller symbol synchronization clocks:
"rx_lane0_sync_clk" - RX Lane 0
"rx_lane1_sync_clk" - RX Lane 1
"tx_lane0_sync_clk" - TX Lane 0
"tx_lane1_sync_clk" - TX Lane 1

Which of the above would your phy_clk refer to?

   Arnd

[1] 
https://android.googlesource.com/kernel/msm/+/android-msm-bullhead-3.10-marshmallow-dr/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt?autodive=0%2F%2F%2F%2F%2F

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

[liwei (CM)]

Hi, Arnd

I'll ask our soc colleagues for help and give a detailed and accurate 
explanation aosp.

Thanks!


-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2018年3月26日 18:42
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); 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; Gengjianfeng; Guodong Xu; Zhangfei Gao; Fengbaopeng 
(kevin, Kirin Solution Dept); Yaniv Gardi
主题: Re: 答复: 答复: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

On Mon, Mar 26, 2018 at 12:26 PM, liwei (CM) <liwei...@huawei.com> wrote:
> 发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd 
> Bergmann
> > 主题: Re: 答复: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for 
> > hisi-ufs On Fri, Mar 23, 2018 at 3:22 AM, liwei (CM) <liwei...@huawei.com> 
> > wrote:
> >> The clock names sound generic enough, should we have both in the generic 
> >> binding?
> >>
> >> Do you mean that add a "phy_clk" to ufshcd-pltfrm 's bindings?
> >> At present, it seems that in the implementation of generic code, 
> >> apart from "ref_clk" may have special processing, other clk will 
> >> not have special processing and simply parse and enable; Referring 
> >> to ufs-qcom binding, I think "phy_clk" can be named "iface_clk", 
> >> this "iface_clk" exists in ufshcd-pltfrm bindings;If so, "ref_clk", 
> >> "iface_clk" are both in the generic binding,we will remove them here. Is 
> >> that okay?
>
> > I'm looking at the generic binding again, and it seems we never 
> > quite managed to fix some minor problems with it. See below for a possible 
> > way to clarify it.
>
> phy_clk is actually given to the phy. But as previously mentioned , we 
> do not have a separate phy to configure ; The clks in the patch you 
> give appear to be unsuitable for describing this .
> Here we can't describe phy_clk in the node "ufsphy1: ufsphy@fc597000" like 
> qcom.
> So can we put it here in our own binding like this?

I think the concept of having a phy clk is generic enough that it's better to 
have that in the common part, others will surely have the same thing, and in 
this case, qcom would be the exception that does not use one.

There are apparently a couple of things related to the phy that may or may not 
require a clk:

- ref_clk: The reference clock on the mipi bus, this is what qcom have, this 
would
  be the 19.2 MHz clock signal.
- one clock to drive the logic block for the PHY itself, if it is included 
within
  the same logical portion of an SoC as the ufshcd, but uses a separate clock.
- Looking at the Android kernel as distributed by google/qualcomm, they have
  four separate clocks described as

PHY to controller symbol synchronization clocks:
"rx_lane0_sync_clk" - RX Lane 0
"rx_lane1_sync_clk" - RX Lane 1
"tx_lane0_sync_clk" - TX Lane 0
"tx_lane1_sync_clk" - TX Lane 1

Which of the above would your phy_clk refer to?

   Arnd

[1] 
https://android.googlesource.com/kernel/msm/+/android-msm-bullhead-3.10-marshmallow-dr/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt?autodive=0%2F%2F%2F%2F%2F

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

[Arnd Bergmann]

On Mon, Mar 26, 2018 at 12:26 PM, liwei (CM) <liwei...@huawei.com> wrote:
> 发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
> > 主题: Re: 答复: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
> > On Fri, Mar 23, 2018 at 3:22 AM, liwei (CM) <liwei...@huawei.com> wrote:
> >> The clock names sound generic enough, should we have both in the generic 
> >> binding?
> >>
> >> Do you mean that add a "phy_clk" to ufshcd-pltfrm 's bindings?
> >> At present, it seems that in the implementation of generic code, apart
> >> from "ref_clk" may have special processing, other clk will not have 
> >> special processing and
> >> simply parse and enable; Referring to ufs-qcom binding, I think "phy_clk" 
> >> can be named
> >> "iface_clk", this "iface_clk" exists in ufshcd-pltfrm bindings;If so, 
> >> "ref_clk", "iface_clk" are
> >> both in the generic binding,we will remove them here. Is that okay?
>
> > I'm looking at the generic binding again, and it seems we never quite 
> > managed to fix some
> > minor problems with it. See below for a possible way to clarify it.
>
> phy_clk is actually given to the phy. But as previously mentioned , we do not 
> have a
> separate phy to configure ; The clks in the patch you give appear to be 
> unsuitable for
> describing this .
> Here we can't describe phy_clk in the node "ufsphy1: ufsphy@fc597000" like 
> qcom.
> So can we put it here in our own binding like this?

I think the concept of having a phy clk is generic enough that it's
better to have
that in the common part, others will surely have the same thing, and
in this case,
qcom would be the exception that does not use one.

There are apparently a couple of things related to the phy that may or may not
require a clk:

- ref_clk: The reference clock on the mipi bus, this is what qcom
have, this would
  be the 19.2 MHz clock signal.
- one clock to drive the logic block for the PHY itself, if it is
included within
  the same logical portion of an SoC as the ufshcd, but uses a separate clock.
- Looking at the Android kernel as distributed by google/qualcomm, they have
  four separate clocks described as

PHY to controller symbol synchronization clocks:
"rx_lane0_sync_clk" - RX Lane 0
"rx_lane1_sync_clk" - RX Lane 1
"tx_lane0_sync_clk" - TX Lane 0
"tx_lane1_sync_clk" - TX Lane 1

Which of the above would your phy_clk refer to?

   Arnd

[1] 
https://android.googlesource.com/kernel/msm/+/android-msm-bullhead-3.10-marshmallow-dr/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt?autodive=0%2F%2F%2F%2F%2F

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

[liwei (CM)]

Hi, Arnd

-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2018年3月26日 17:14
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); 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; Gengjianfeng; Guodong Xu; Zhangfei Gao; Fengbaopeng 
(kevin, Kirin Solution Dept); Yaniv Gardi
主题: Re: 答复: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

On Fri, Mar 23, 2018 at 3:22 AM, liwei (CM) <liwei...@huawei.com> wrote:
>> diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> new file mode 100644
>> index ..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?
>
> Do you mean that add a "phy_clk" to ufshcd-pltfrm 's bindings?
> At present, it seems that in the implementation of generic code, apart 
> from "ref_clk" may have special processing, other clk will not have special 
> processing and simply parse and enable; Referring to ufs-qcom binding, I 
> think "phy_clk" can be named "iface_clk", this "iface_clk" exists in 
> ufshcd-pltfrm bindings;If so, "ref_clk", "iface_clk" are both in the generic 
> binding,we will remove them here. Is that okay?

I'm looking at the generic binding again, and it seems we never quite managed 
to fix some minor problems with it. See below for a possible way to clarify it.

phy_clk is actually given to the phy. But as previously mentioned , we do not 
have a separate phy to configure ; The clks in the patch you give appear to be 
unsuitable for describing this .
Here we can't describe phy_clk in the node "ufsphy1: ufsphy@fc597000" like 
qcom. So can we put it here in our own binding like this?

>> +- 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.
>
> 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 I think it may be difficult to add this to common code, or it may 
> not be necessary; Other manufacturers may not need to do this soc init 
> because they probably already done in the bootloader phase. Even if they need 
> to do it, it's probably different from us.
> We need to make sure that our ufs works even if not do soc init during the 
> bootloader phase.

In the suggested patch below, I have documented one "rst" line that is used to 
reset the ufshcd device. The second reset line as I understand now is used in a 
rather nonstandard way and gets asserted only while setting up the additional 
registers for your glue logic, so that one seems better left documented in your 
own binding.

Yes, the second reset line is used in a rather nonstandard way , if rst will 
into the common document, ,I will left the second reset line documented in our 
own binding.

I've added a "jedec,ufshci-3.0" compatible string, which appears to be the 
latest version of the ufshci itself, and I've documented four clocks that are 
already used by the qualcomm var

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

[Arnd Bergmann]

On Fri, Mar 23, 2018 at 3:22 AM, liwei (CM)  wrote:
>> diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> new file mode 100644
>> index ..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?
>
> Do you mean that add a "phy_clk" to ufshcd-pltfrm 's bindings?
> At present, it seems that in the implementation of generic code, apart from 
> "ref_clk" may have special processing, other clk will not have special 
> processing and simply parse and enable;
> Referring to ufs-qcom binding, I think "phy_clk" can be named "iface_clk", 
> this "iface_clk" exists in ufshcd-pltfrm bindings;If so, "ref_clk", 
> "iface_clk" are both in the generic binding,we will remove them here. Is that 
> okay?

I'm looking at the generic binding again, and it seems we never quite
managed to fix
some minor problems with it. See below for a possible way to clarify it.

>> +- 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.
>
> 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 I think it may be difficult to add this to common code, or it may not be 
> necessary;
> Other manufacturers may not need to do this soc init because they probably 
> already done in the bootloader phase. Even if they need to do it, it's 
> probably different from us.
> We need to make sure that our ufs works even if not do soc init during the 
> bootloader phase.

In the suggested patch below, I have documented one "rst" line that is used to
reset the ufshcd device. The second reset line as I understand now is used in
a rather nonstandard way and gets asserted only while setting up the additional
registers for your glue logic, so that one seems better left documented in your
own binding.

I've added a "jedec,ufshci-3.0" compatible string, which appears to be
the latest
version of the ufshci itself, and I've documented four clocks that are
already used
by the qualcomm variant of the platform device. Please have a look at the below,
and see if we need additional changes or clarifications. With this, most of your
binding can get folded into the common document, so you just need to explain
the private compatible string, the larger register area, and the
additional reset line.

  Arnd

commit a945e9bc823521253c9ff5a061f22a2aa7fd335e
Author: Arnd Bergmann 
Date:   Mon Mar 26 11:07:46 2018 +0200

ufshcd: clarify some parts of the documentation

Signed-off-by: Arnd Bergmann 

diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index c39dfef76a18..bc90a7d8385b 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -4,8 +4,10 @@ UFSHC nodes are defined to describe on-chip UFS host
controllers.
 Each UFS controller instance should have its own node.

 Required properties:
-- compatible   : must contain "jedec,ufs-1.1" or "jedec,ufs-2.0", may
- also list one or more of the following:
+- compatible   : must contain "jedec,ufs-1.1", "jedec,ufs-2.0",
+ or "jedec,ufshci-3.0", and may also list one
+ or more of the following, as well as others
+ specified in derived bindings:
  "qcom,msm8994-ufshc"
  "qcom,msm8996-ufshc"
   

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

[liwei (CM)]

Hi, Arnd
Sorry to bother you again, please take the time to review the patch. Are there 
any other suggestions?
Looking forward to your reply.

-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2018年2月19日 17:58
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); 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; Gengjianfeng; Guodong Xu; Zhangfei Gao; Fengbaopeng 
(kevin, Kirin Solution Dept)
主题: Re: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

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 ..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?

Do you mean that add a "phy_clk" to ufshcd-pltfrm 's bindings? 
At present, it seems that in the implementation of generic code, apart from 
"ref_clk" may have special processing, other clk will not have special 
processing and simply parse and enable;
Referring to ufs-qcom binding, I think "phy_clk" can be named "iface_clk", this 
"iface_clk" exists in ufshcd-pltfrm bindings;If so, "ref_clk", "iface_clk" are 
both in the generic binding,we will remove them here. Is that okay?

> +- 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.

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 I think it may be difficult to add this to common code, or it may not be 
necessary; 
Other manufacturers may not need to do this soc init because they probably 
already done in the bootloader phase. Even if they need to do it, it's probably 
different from us.
We need to make sure that our ufs works even if not do soc init during the 
bootloader phase.




  Arnd

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

[liwei (CM)]

Hi，Arnd

Sorry to bother you again, please take the time to review the patch. Are there 
any other suggestions?
Looking forward to your reply.

Thanks!



-邮件原件-
发件人: liwei (CM) 
发送时间: 2018年2月23日 16:36
收件人: 'Arnd Bergmann'
抄送: Rob Herring; Mark Rutland; xuwei (O); 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; DTML; Linux Kernel Mailing List; Linux ARM; linux-scsi; 
zangleigang; Gengjianfeng; Guodong Xu; Zhangfei Gao; Fengbaopeng (kevin, Kirin 
Solution Dept)
主题: 答复: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

Hi, Arnd

Sorry late for you.
The following two suggestions we have really discussed
https://lkml.org/lkml/2017/11/30/1077

-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2018年2月19日 17:58
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); 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; Gengjianfeng; Guodong Xu; Zhangfei Gao; Fengbaopeng 
(kevin, Kirin Solution Dept)
主题: Re: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

On Tue, Feb 13, 2018 at 11:14 AM, Li Wei <liwei...@huawei.com> wrote:
> add ufs node document for Hisilicon.
>
> Signed-off-by: Li Wei <liwei...@huawei.com>
> ---
>  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 ..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?

"ref_clk" is in the ufshcd-pltfrm binding, but "phy_clk" is not; what do you 
mean is that these two don't need to document here?

> +- 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.

As discussed in https://lkml.org/lkml/2017/11/30/1077;
If document it in the ufshcd binding, I think it needs some codes to parse them 
in ufshcd.c/ufshcd-pltfrm.c, but I think these codes may not be applicable to 
other SOC manufacturers.

  Arnd

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

[liwei (CM)]

Hi, Arnd

Sorry late for you.
The following two suggestions we have really discussed
https://lkml.org/lkml/2017/11/30/1077

-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2018年2月19日 17:58
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); 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; Gengjianfeng; Guodong Xu; Zhangfei Gao; Fengbaopeng 
(kevin, Kirin Solution Dept)
主题: Re: [PATCH v8 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

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 ..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?

"ref_clk" is in the ufshcd-pltfrm binding, but "phy_clk" is not; what do you 
mean is that these two don't need to document here?

> +- 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.

As discussed in https://lkml.org/lkml/2017/11/30/1077;
If document it in the ufshcd binding, I think it needs some codes to parse them 
in ufshcd.c/ufshcd-pltfrm.c, but I think these codes may not be applicable to 
other SOC manufacturers.

  Arnd