Hi Heiko, Doug
在 2016年12月17日 01:28, Doug Anderson 写道:
Hi,
On Thu, Dec 15, 2016 at 10:57 PM, Xing Zheng <zhengx...@rock-chips.com> wrote:
Hi Heiko, Doug,
On 2016年12月16日 02:18, Heiko Stuebner wrote:
Am Donnerstag, 15. Dezember 2016, 08:34:09 CET schrieb Doug Anderson:
I still need to digest all of the things that were added to this
thread overnight, but nothing I've seen so far indicates that you need
the post-gated clock. AKA I still think you need to redo your patch
to replace:
clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
<&cru SCLK_USBPHY0_480M_SRC>;
with:
clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
<&u2phy0>;
Can you please comment on that?
Also, with the change, the ehci will keep the clock (and thus the phy)
always
on. Does the phy-autosuspend even save anything now?
In any case, could we make the clock-names entry sound nicer than
usbphy0_480m
please? bindings/usb/atmel-usb.txt calls its UTMI clock simply "usb_clk",
but
something like "utmi" should also work.
While at it you could also fix up the other clock names to something like
"host" and "arbiter" or so?.
Heiko
The usbphy related clock tress like this:
Actually, at drivers/phy/phy-rockchip-inno-usb2.c, we can only
enable/disable the master gate via GRF is PHY_PLL, not UTMI_CLK.
And the naming style of the "hclk_host0" keep the name "hclk_host0" on the
clcok tree diagram:
Therefore, could we rename the clock name like this:
----
for usb_host0_ehci and usb_host0_ohci:
clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
<&cru SCLK_U2PHY0>;
clock-names = "hclk_host0", "hclk_host0_arb",
"sclk_u2phy0";
for usb_host1_ehci and usb_host1_ohci:
clocks = <&cru HCLK_HOST1>, <&cru HCLK_HOST1_ARB>,
<&cru SCLK_U2PHY1>;
clock-names = "hclk_host1", "hclk_host1_arb",
"sclk_u2phy1";
----
BTW, the "arb" is an abbreviation for arbiter.
You don't specify what this new "SCLK_U2PHY0" ID is, so it's a little
hard for me to know what you're intending.
...however, I still don't see any reason why you can't just use the
solution I proposed. Specifying the clock as "<&u2phy0>" is the
correct thing to do. The input clock to the EHCI driver is exactly
the clock provided by the USB PHY with no gate in between (just as I
said). There is no reason to somehow buffer it by the cru. The cru
doesn't see this clock and has no reason to be involved.
Note that there were many other comments on this thread besides mine.
Are you planning to address any of them?
-Doug
Done, and have resent the patch.
Thanks.
--
- Xing Zheng
