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!
-----邮件原件-----
发件人: [email protected] [mailto:[email protected]] 代表 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) <[email protected]> wrote:
> 发件人: [email protected] [mailto:[email protected]] 代表 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) <[email protected]> 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
On Tue, Mar 27, 2018 at 8:15 AM, liwei (CM) <[email protected]> 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
Hi, Arnd
Thanks for your patiences.
-----邮件原件-----
发件人: [email protected] [mailto:[email protected]] 代表 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) <[email protected]> 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