Hi James, On Fri, May 22, 2015 at 1:40 PM, James Liao <jamesjj.liao@xxxxxxxxxxxx> wrote: > Hi Daniel, > > On Fri, 2015-05-22 at 12:19 +0800, Daniel Kurtz wrote: >> On Fri, May 22, 2015 at 10:41 AM, James Liao <jamesjj.liao@xxxxxxxxxxxx> wrote: >> > >> > 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. > >> > >> > 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 > >> 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. > >> 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 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html