Hi James,

On Fri, May 22, 2015 at 1:40 PM, James Liao [off-list ref] wrote:

Hi Daniel,

On Fri, 2015-05-22 at 12:19 +0800, Daniel Kurtz wrote:

quoted

On Fri, May 22, 2015 at 10:41 AM, James Liao [off-list ref] wrote:

quoted

Sascha is right. I had confirmed with our designer that MFG on MT8173
uses clk26m to check state. I also tested MFG domain power on/off with
CLK_TOP_MFG_SEL off and it worked correctly.

Wait - the designer said to use clk26m, but you tested with CLK_TOP_MFG_SEL.
Now I am even more confused.
Which is the correct clock to use for the mfg power domains?

I tested MFG domain power on/off with MFG_SEL "OFF". If MFG domain use
MFG_SEL as its bus clock, it will be timeout while waiting SRAM ACK. But
in my test, MFG power domain can power on/off without MFG_SEL. That
means it doesn't need MFG_SEL while power on/off MFG domains.

quoted

quoted

My patch set yesterday add subsystem clocks, which are not needed by
power domain on/off. So I think these 2 patch set are independent.

In other versions of the SCPSYS patch set, the scpsys node has
additional "subsystem bus clocks".

That's my fault because I provided wrong information to Sascha. So in
his early version of mtk-scpsys driver, the clocks setting of power
domains are incorrect. The correct clock setting for each power domain
should be:

POWER_DOMAIN_MFG:     clk26m (no need to enable topckgen clocks)
POWER_DOMAIN_DISP:    mm_sel
POWER_DOMAIN_VDEC:    mm_sel
POWER_DOMAIN_VENC:    mm_sel
POWER_DOMAIN_VENC_LT: mm_sel
POWER_DOMAIN_ISP:     mm_sel

quoted

So will we need additional patches later to add back these additional
clocks which have been removed from the current version of this pach?

No. Clocks provided by topckgen are enough for scpsys driver. Please see
my comments below.

quoted

In other words, will there be a follow up patch like below, plus
another patch to add these clocks to "enum clk_id":

@@ -163,9 +163,12 @@
  compatible = "mediatek,mt8173-scpsys";
  #power-domain-cells = <1>;
  reg = <0 0x10006000 0 0x1000>;
- clocks = <&clk26m>,
- <&topckgen CLK_TOP_MM_SEL>;
- clock-names = "mfg", "mm";
+ clocks = <&topckgen CLK_TOP_MFG_SEL>,
+ <&topckgen CLK_TOP_MM_SEL>,
+ <&topckgen CLK_TOP_VDEC_SEL>,
+ <&topckgen CLK_TOP_VENC_SEL>,
+ <&topckgen CLK_TOP_VENC_LT_SEL>;
+ clock-names = "mfg", "mm", "vdec", "venc", "venc_lt";
  infracfg = <&infracfg>;
  };

These clocks come from topckgen. In our term, they are "top clocks".
Subsystem clocks which mentioned in my patch set are provided by
subsystems such as mmsys, vdecsys, vencsys and so on.

This is all great news!
Thanks for the detailed update.

Best Regards,
-Daniel

Best regards,

James