On Thu, Mar 12, 2020 at 04:04:57PM +0530, Anand Moon wrote:

Hi Krzysztof,

Thanks for your review comments.

On Wed, 11 Mar 2020 at 20:12, Krzysztof Kozlowski [off-list ref] wrote:

quoted

On Tue, Mar 10, 2020 at 07:48:54PM +0000, Anand Moon wrote:

quoted

FSYS power domain support usbdrd3, pdma and usb2 power gaiting,
hence move FSYS clk setting to sub-CMU block to support power domain
on/off sequences for device nodes.

Signed-off-by: Anand Moon <redacted>
---
New patch in the series
---
 drivers/clk/samsung/clk-exynos5420.c | 45 +++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index c9e5a1fb6653..6c4c47dfcdce 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c

@@ -859,12 +859,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
      DIV(0, "dout_maudio0", "mout_maudio0", DIV_MAU, 20, 4),
      DIV(0, "dout_maupcm0", "dout_maudio0", DIV_MAU, 24, 8),

-     /* USB3.0 */
-     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
-     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
-     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
-     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),

According to clock diagram these are still in CMU TOP, not FSYS.

quoted

-
      /* MMC */
      DIV(0, "dout_mmc0", "mout_mmc0", DIV_FSYS1, 0, 10),
      DIV(0, "dout_mmc1", "mout_mmc1", DIV_FSYS1, 10, 10),

@@ -1031,8 +1025,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {

/>

quoted

      /* FSYS Block */
      GATE(CLK_TSI, "tsi", "aclk200_fsys", GATE_BUS_FSYS0, 0, 0, 0),
-     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
-     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
      GATE(CLK_UFS, "ufs", "aclk200_fsys2", GATE_BUS_FSYS0, 3, 0, 0),
      GATE(CLK_RTIC, "rtic", "aclk200_fsys", GATE_IP_FSYS, 9, 0, 0),
      GATE(CLK_MMC0, "mmc0", "aclk200_fsys2", GATE_IP_FSYS, 12, 0, 0),

@@ -1040,9 +1032,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
      GATE(CLK_MMC2, "mmc2", "aclk200_fsys2", GATE_IP_FSYS, 14, 0, 0),
      GATE(CLK_SROMC, "sromc", "aclk200_fsys2",
                      GATE_IP_FSYS, 17, CLK_IGNORE_UNUSED, 0),
-     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
-     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
-     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
      GATE(CLK_SCLK_UNIPRO, "sclk_unipro", "dout_unipro",
                      SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),

@@ -1258,6 +1247,28 @@ static struct exynos5_subcmu_reg_dump exynos5x_gsc_suspend_regs[] = {
      { DIV2_RATIO0, 0, 0x30 },       /* DIV dout_gscl_blk_300 */
 };

+/* USB3.0 */
+static const struct samsung_div_clock exynos5x_fsys_div_clks[] __initconst = {
+     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
+     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
+     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
+     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
+};
+
+static const struct samsung_gate_clock exynos5x_fsys_gate_clks[] __initconst = {
+     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
+     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
+     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
+     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
+     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
+};
+
+static struct exynos5_subcmu_reg_dump exynos5x_fsys_suspend_regs[] = {
+     { GATE_IP_FSYS, 0xffffffff, 0xffffffff }, /* FSYS gates */

This looks wrong. GATE_IP_FSYS has fields also for FSYS2 clocks which
you are not suspending. They do not belong to this CMU.

Ok. I change the from GATE_IP_FSYS to GATE_BUS_FSYS0 in the above
exynos5x_fsys_gate_clks to make this consistent to used GATE_BUS_FSYS0 for CMU,
with this change it works as per previously.

Wait, you should set here proper registers with proper mask.

quoted

Don't you need to save also parts of GATE_BUS_FSYS0?

GATE_BUS_FSYS0 and GATE_IP_FSYS are already part of list
of control register which are saved and restored during suspend and resume
so no point in adding this here, I should drop the GATE_IP_FSYS reg
dump over here.

Since registers are used in separate sub CMU devices, each should
save/restore its part.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel