[PATCH 0/4] ARM: rockchip: add dma support

[PATCH 0/4] ARM: rockchip: add dma support

[Heiko Stuebner]

All Rockchip SoCs currently supported use pl330 dma controllers.
The first patch introduces the concept of critical clocks, stolen from
sunxi, as some core clocks shouldn't be disabled under normal circumstances.
The patch is necessary, as the amba bus uses strict clock gating, which
without this patch results in the core aclk getting disabled halting the
system, before other components are able to probe and maybe claim their
clocks.

As the patches have no compile-time dependency on each other, the patchset
could be split with the first patch going through the clock tree and the
other three going through arm-soc.

Heiko Stuebner (4):
  clk: rockchip: protect critical clocks from getting disabled
  ARM: rockchip: enable the AMBA bus
  ARM: dts: rockchip: add rk3288 dma controllers
  ARM: dts: rockchip: add rk3188 dma controllers

 arch/arm/boot/dts/rk3288.dtsi     | 27 +++++++++++++++++++++++++++
 arch/arm/boot/dts/rk3xxx.dtsi     | 27 +++++++++++++++++++++++++++
 arch/arm/mach-rockchip/Kconfig    |  1 +
 drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
 drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
 drivers/clk/rockchip/clk.c        | 13 +++++++++++++
 drivers/clk/rockchip/clk.h        |  1 +
 7 files changed, 83 insertions(+)

-- 
2.0.1

[PATCH 0/4] ARM: rockchip: add dma support

[Heiko Stuebner]

All Rockchip SoCs currently supported use pl330 dma controllers.
The first patch introduces the concept of critical clocks, stolen from
sunxi, as some core clocks shouldn't be disabled under normal circumstances.
The patch is necessary, as the amba bus uses strict clock gating, which
without this patch results in the core aclk getting disabled halting the
system, before other components are able to probe and maybe claim their
clocks.

As the patches have no compile-time dependency on each other, the patchset
could be split with the first patch going through the clock tree and the
other three going through arm-soc.

Heiko Stuebner (4):
  clk: rockchip: protect critical clocks from getting disabled
  ARM: rockchip: enable the AMBA bus
  ARM: dts: rockchip: add rk3288 dma controllers
  ARM: dts: rockchip: add rk3188 dma controllers

 arch/arm/boot/dts/rk3288.dtsi     | 27 +++++++++++++++++++++++++++
 arch/arm/boot/dts/rk3xxx.dtsi     | 27 +++++++++++++++++++++++++++
 arch/arm/mach-rockchip/Kconfig    |  1 +
 drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
 drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
 drivers/clk/rockchip/clk.c        | 13 +++++++++++++
 drivers/clk/rockchip/clk.h        |  1 +
 7 files changed, 83 insertions(+)

-- 
2.0.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stuebner]

The clock-tree contains clocks that should never get disabled automatically.
One example are the base ACLKs, the base supplies for all peripherals.

Therefore add a structure similar to the sunxi clock-tree to protect these
special clocks from being disabled.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
 drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
 drivers/clk/rockchip/clk.c        | 13 +++++++++++++
 drivers/clk/rockchip/clk.h        |  1 +
 4 files changed, 28 insertions(+)

diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
index a83a6d8..5aef277 100644
--- a/drivers/clk/rockchip/clk-rk3188.c
+++ b/drivers/clk/rockchip/clk-rk3188.c
@@ -599,6 +599,11 @@ static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = {
 	GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),
 };
 
+static const char *rk3188_critical_clocks[] __initconst = {
+	"aclk_cpu",
+	"aclk_peri",
+};
+
 static void __init rk3188_common_clk_init(struct device_node *np)
 {
 	void __iomem *reg_base;
@@ -628,6 +633,8 @@ static void __init rk3188_common_clk_init(struct device_node *np)
 				   RK3188_GRF_SOC_STATUS);
 	rockchip_clk_register_branches(common_clk_branches,
 				  ARRAY_SIZE(common_clk_branches));
+	rockchip_clk_protect_critical(rk3188_critical_clocks,
+				      ARRAY_SIZE(rk3188_critical_clocks));
 
 	rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
 				  ROCKCHIP_SOFTRST_HIWORD_MASK);
diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index 0d8c6c5..6c6f954 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -680,6 +680,11 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
 	GATE(0, "pclk_isp_in", "ext_isp", 0, RK3288_CLKGATE_CON(16), 3, GFLAGS),
 };
 
+static const char *rk3288_critical_clocks[] __initconst = {
+	"aclk_cpu",
+	"aclk_peri",
+};
+
 static void __init rk3288_clk_init(struct device_node *np)
 {
 	void __iomem *reg_base;
@@ -710,6 +715,8 @@ static void __init rk3288_clk_init(struct device_node *np)
 				   RK3288_GRF_SOC_STATUS);
 	rockchip_clk_register_branches(rk3288_clk_branches,
 				  ARRAY_SIZE(rk3288_clk_branches));
+	rockchip_clk_protect_critical(rk3288_critical_clocks,
+				      ARRAY_SIZE(rk3288_critical_clocks));
 
 	rockchip_register_softrst(np, 9, reg_base + RK3288_SOFTRST_CON(0),
 				  ROCKCHIP_SOFTRST_HIWORD_MASK);
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index 278cf9d..9189f1b 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -242,3 +242,16 @@ void __init rockchip_clk_register_branches(
 		rockchip_clk_add_lookup(clk, list->id);
 	}
 }
+
+void __init rockchip_clk_protect_critical(const char *clocks[], int nclocks)
+{
+	int i;
+
+	/* Protect the clocks that needs to stay on */
+	for (i = 0; i < nclocks; i++) {
+		struct clk *clk = __clk_lookup(clocks[i]);
+
+		if (clk)
+			clk_prepare_enable(clk);
+	}
+}
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index 887cbde..2b0bca1 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -329,6 +329,7 @@ void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list,
 				    unsigned int nr_clk);
 void rockchip_clk_register_plls(struct rockchip_pll_clock *pll_list,
 				unsigned int nr_pll, int grf_lock_offset);
+void rockchip_clk_protect_critical(const char *clocks[], int nclocks);
 
 #define ROCKCHIP_SOFTRST_HIWORD_MASK	BIT(0)
 
-- 
2.0.1

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stuebner]

The clock-tree contains clocks that should never get disabled automatically.
One example are the base ACLKs, the base supplies for all peripherals.

Therefore add a structure similar to the sunxi clock-tree to protect these
special clocks from being disabled.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
 drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
 drivers/clk/rockchip/clk.c        | 13 +++++++++++++
 drivers/clk/rockchip/clk.h        |  1 +
 4 files changed, 28 insertions(+)

diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
index a83a6d8..5aef277 100644
--- a/drivers/clk/rockchip/clk-rk3188.c
+++ b/drivers/clk/rockchip/clk-rk3188.c
@@ -599,6 +599,11 @@ static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = {
 	GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),
 };
 
+static const char *rk3188_critical_clocks[] __initconst = {
+	"aclk_cpu",
+	"aclk_peri",
+};
+
 static void __init rk3188_common_clk_init(struct device_node *np)
 {
 	void __iomem *reg_base;
@@ -628,6 +633,8 @@ static void __init rk3188_common_clk_init(struct device_node *np)
 				   RK3188_GRF_SOC_STATUS);
 	rockchip_clk_register_branches(common_clk_branches,
 				  ARRAY_SIZE(common_clk_branches));
+	rockchip_clk_protect_critical(rk3188_critical_clocks,
+				      ARRAY_SIZE(rk3188_critical_clocks));
 
 	rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
 				  ROCKCHIP_SOFTRST_HIWORD_MASK);
diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index 0d8c6c5..6c6f954 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -680,6 +680,11 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
 	GATE(0, "pclk_isp_in", "ext_isp", 0, RK3288_CLKGATE_CON(16), 3, GFLAGS),
 };
 
+static const char *rk3288_critical_clocks[] __initconst = {
+	"aclk_cpu",
+	"aclk_peri",
+};
+
 static void __init rk3288_clk_init(struct device_node *np)
 {
 	void __iomem *reg_base;
@@ -710,6 +715,8 @@ static void __init rk3288_clk_init(struct device_node *np)
 				   RK3288_GRF_SOC_STATUS);
 	rockchip_clk_register_branches(rk3288_clk_branches,
 				  ARRAY_SIZE(rk3288_clk_branches));
+	rockchip_clk_protect_critical(rk3288_critical_clocks,
+				      ARRAY_SIZE(rk3288_critical_clocks));
 
 	rockchip_register_softrst(np, 9, reg_base + RK3288_SOFTRST_CON(0),
 				  ROCKCHIP_SOFTRST_HIWORD_MASK);
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index 278cf9d..9189f1b 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -242,3 +242,16 @@ void __init rockchip_clk_register_branches(
 		rockchip_clk_add_lookup(clk, list->id);
 	}
 }
+
+void __init rockchip_clk_protect_critical(const char *clocks[], int nclocks)
+{
+	int i;
+
+	/* Protect the clocks that needs to stay on */
+	for (i = 0; i < nclocks; i++) {
+		struct clk *clk = __clk_lookup(clocks[i]);
+
+		if (clk)
+			clk_prepare_enable(clk);
+	}
+}
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index 887cbde..2b0bca1 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -329,6 +329,7 @@ void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list,
 				    unsigned int nr_clk);
 void rockchip_clk_register_plls(struct rockchip_pll_clock *pll_list,
 				unsigned int nr_pll, int grf_lock_offset);
+void rockchip_clk_protect_critical(const char *clocks[], int nclocks);
 
 #define ROCKCHIP_SOFTRST_HIWORD_MASK	BIT(0)
 
-- 
2.0.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Mike Turquette]

Quoting Heiko Stuebner (2014-07-29 12:12:05)
> The clock-tree contains clocks that should never get disabled automatically.
> One example are the base ACLKs, the base supplies for all peripherals.
> 
> Therefore add a structure similar to the sunxi clock-tree to protect these
> special clocks from being disabled.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
>  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
>  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
>  drivers/clk/rockchip/clk.h        |  1 +
>  4 files changed, 28 insertions(+)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
> index a83a6d8..5aef277 100644
> --- a/drivers/clk/rockchip/clk-rk3188.c
> +++ b/drivers/clk/rockchip/clk-rk3188.c
> @@ -599,6 +599,11 @@ static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = {
>         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),
>  };
>  
> +static const char *rk3188_critical_clocks[] __initconst = {
> +       "aclk_cpu",
> +       "aclk_peri",

I'm not against the idea of critical clocks, but I want to verify that
there is no other driver out there that is a better fit for claiming
these clks via clk_get and enabling them the normal way via clk_enable?

Regards,
Mike

> +};
> +
>  static void __init rk3188_common_clk_init(struct device_node *np)
>  {
>         void __iomem *reg_base;
> @@ -628,6 +633,8 @@ static void __init rk3188_common_clk_init(struct device_node *np)
>                                    RK3188_GRF_SOC_STATUS);
>         rockchip_clk_register_branches(common_clk_branches,
>                                   ARRAY_SIZE(common_clk_branches));
> +       rockchip_clk_protect_critical(rk3188_critical_clocks,
> +                                     ARRAY_SIZE(rk3188_critical_clocks));
>  
>         rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
>                                   ROCKCHIP_SOFTRST_HIWORD_MASK);
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index 0d8c6c5..6c6f954 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -680,6 +680,11 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
>         GATE(0, "pclk_isp_in", "ext_isp", 0, RK3288_CLKGATE_CON(16), 3, GFLAGS),
>  };
>  
> +static const char *rk3288_critical_clocks[] __initconst = {
> +       "aclk_cpu",
> +       "aclk_peri",
> +};
> +
>  static void __init rk3288_clk_init(struct device_node *np)
>  {
>         void __iomem *reg_base;
> @@ -710,6 +715,8 @@ static void __init rk3288_clk_init(struct device_node *np)
>                                    RK3288_GRF_SOC_STATUS);
>         rockchip_clk_register_branches(rk3288_clk_branches,
>                                   ARRAY_SIZE(rk3288_clk_branches));
> +       rockchip_clk_protect_critical(rk3288_critical_clocks,
> +                                     ARRAY_SIZE(rk3288_critical_clocks));
>  
>         rockchip_register_softrst(np, 9, reg_base + RK3288_SOFTRST_CON(0),
>                                   ROCKCHIP_SOFTRST_HIWORD_MASK);
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 278cf9d..9189f1b 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -242,3 +242,16 @@ void __init rockchip_clk_register_branches(
>                 rockchip_clk_add_lookup(clk, list->id);
>         }
>  }
> +
> +void __init rockchip_clk_protect_critical(const char *clocks[], int nclocks)
> +{
> +       int i;
> +
> +       /* Protect the clocks that needs to stay on */
> +       for (i = 0; i < nclocks; i++) {
> +               struct clk *clk = __clk_lookup(clocks[i]);
> +
> +               if (clk)
> +                       clk_prepare_enable(clk);
> +       }
> +}
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index 887cbde..2b0bca1 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -329,6 +329,7 @@ void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list,
>                                     unsigned int nr_clk);
>  void rockchip_clk_register_plls(struct rockchip_pll_clock *pll_list,
>                                 unsigned int nr_pll, int grf_lock_offset);
> +void rockchip_clk_protect_critical(const char *clocks[], int nclocks);
>  
>  #define ROCKCHIP_SOFTRST_HIWORD_MASK   BIT(0)
>  
> -- 
> 2.0.1
> 

Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Mike Turquette]

Quoting Heiko Stuebner (2014-07-29 12:12:05)
> The clock-tree contains clocks that should never get disabled automatically.
> One example are the base ACLKs, the base supplies for all peripherals.
> 
> Therefore add a structure similar to the sunxi clock-tree to protect these
> special clocks from being disabled.
> 
> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
>  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
>  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
>  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
>  drivers/clk/rockchip/clk.h        |  1 +
>  4 files changed, 28 insertions(+)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
> index a83a6d8..5aef277 100644
> --- a/drivers/clk/rockchip/clk-rk3188.c
> +++ b/drivers/clk/rockchip/clk-rk3188.c
> @@ -599,6 +599,11 @@ static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = {
>         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),
>  };
>  
> +static const char *rk3188_critical_clocks[] __initconst = {
> +       "aclk_cpu",
> +       "aclk_peri",

I'm not against the idea of critical clocks, but I want to verify that
there is no other driver out there that is a better fit for claiming
these clks via clk_get and enabling them the normal way via clk_enable?

Regards,
Mike

> +};
> +
>  static void __init rk3188_common_clk_init(struct device_node *np)
>  {
>         void __iomem *reg_base;
> @@ -628,6 +633,8 @@ static void __init rk3188_common_clk_init(struct device_node *np)
>                                    RK3188_GRF_SOC_STATUS);
>         rockchip_clk_register_branches(common_clk_branches,
>                                   ARRAY_SIZE(common_clk_branches));
> +       rockchip_clk_protect_critical(rk3188_critical_clocks,
> +                                     ARRAY_SIZE(rk3188_critical_clocks));
>  
>         rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
>                                   ROCKCHIP_SOFTRST_HIWORD_MASK);
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index 0d8c6c5..6c6f954 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -680,6 +680,11 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
>         GATE(0, "pclk_isp_in", "ext_isp", 0, RK3288_CLKGATE_CON(16), 3, GFLAGS),
>  };
>  
> +static const char *rk3288_critical_clocks[] __initconst = {
> +       "aclk_cpu",
> +       "aclk_peri",
> +};
> +
>  static void __init rk3288_clk_init(struct device_node *np)
>  {
>         void __iomem *reg_base;
> @@ -710,6 +715,8 @@ static void __init rk3288_clk_init(struct device_node *np)
>                                    RK3288_GRF_SOC_STATUS);
>         rockchip_clk_register_branches(rk3288_clk_branches,
>                                   ARRAY_SIZE(rk3288_clk_branches));
> +       rockchip_clk_protect_critical(rk3288_critical_clocks,
> +                                     ARRAY_SIZE(rk3288_critical_clocks));
>  
>         rockchip_register_softrst(np, 9, reg_base + RK3288_SOFTRST_CON(0),
>                                   ROCKCHIP_SOFTRST_HIWORD_MASK);
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 278cf9d..9189f1b 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -242,3 +242,16 @@ void __init rockchip_clk_register_branches(
>                 rockchip_clk_add_lookup(clk, list->id);
>         }
>  }
> +
> +void __init rockchip_clk_protect_critical(const char *clocks[], int nclocks)
> +{
> +       int i;
> +
> +       /* Protect the clocks that needs to stay on */
> +       for (i = 0; i < nclocks; i++) {
> +               struct clk *clk = __clk_lookup(clocks[i]);
> +
> +               if (clk)
> +                       clk_prepare_enable(clk);
> +       }
> +}
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index 887cbde..2b0bca1 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -329,6 +329,7 @@ void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list,
>                                     unsigned int nr_clk);
>  void rockchip_clk_register_plls(struct rockchip_pll_clock *pll_list,
>                                 unsigned int nr_pll, int grf_lock_offset);
> +void rockchip_clk_protect_critical(const char *clocks[], int nclocks);
>  
>  #define ROCKCHIP_SOFTRST_HIWORD_MASK   BIT(0)
>  
> -- 
> 2.0.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stübner]

Hi Mike,

Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> Quoting Heiko Stuebner (2014-07-29 12:12:05)
> 
> > The clock-tree contains clocks that should never get disabled
> > automatically. One example are the base ACLKs, the base supplies for all
> > peripherals.
> > 
> > Therefore add a structure similar to the sunxi clock-tree to protect these
> > special clocks from being disabled.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > 
> >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> >  drivers/clk/rockchip/clk.h        |  1 +
> >  4 files changed, 28 insertions(+)
> > 
> > diff --git a/drivers/clk/rockchip/clk-rk3188.c
> > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> > --- a/drivers/clk/rockchip/clk-rk3188.c
> > +++ b/drivers/clk/rockchip/clk-rk3188.c
> > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> > rk3188_clk_branches[] __initdata = {> 
> >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8),
> >         13, GFLAGS),>  
> >  };
> > 
> > +static const char *rk3188_critical_clocks[] __initconst = {
> > +       "aclk_cpu",
> > +       "aclk_peri",
> 
> I'm not against the idea of critical clocks, but I want to verify that
> there is no other driver out there that is a better fit for claiming
> these clks via clk_get and enabling them the normal way via clk_enable?

In the clock hierarchy of Rockchip SoCs, both aclks listed here, are sources 
for pclk and hclk, as well as sourcing some other peripheral gates further 
below too. So from what I've seen from the clock diagrams, there is nothing 
that would claim these clocks directly, and it wouldn't also make any sense to 
let them get disabled as there will always be something using them (for 
example the dram-controller).


Heiko

Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stübner]

Hi Mike,

Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> Quoting Heiko Stuebner (2014-07-29 12:12:05)
> 
> > The clock-tree contains clocks that should never get disabled
> > automatically. One example are the base ACLKs, the base supplies for all
> > peripherals.
> > 
> > Therefore add a structure similar to the sunxi clock-tree to protect these
> > special clocks from being disabled.
> > 
> > Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> > ---
> > 
> >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> >  drivers/clk/rockchip/clk.h        |  1 +
> >  4 files changed, 28 insertions(+)
> > 
> > diff --git a/drivers/clk/rockchip/clk-rk3188.c
> > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> > --- a/drivers/clk/rockchip/clk-rk3188.c
> > +++ b/drivers/clk/rockchip/clk-rk3188.c
> > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> > rk3188_clk_branches[] __initdata = {> 
> >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8),
> >         13, GFLAGS),>  
> >  };
> > 
> > +static const char *rk3188_critical_clocks[] __initconst = {
> > +       "aclk_cpu",
> > +       "aclk_peri",
> 
> I'm not against the idea of critical clocks, but I want to verify that
> there is no other driver out there that is a better fit for claiming
> these clks via clk_get and enabling them the normal way via clk_enable?

In the clock hierarchy of Rockchip SoCs, both aclks listed here, are sources 
for pclk and hclk, as well as sourcing some other peripheral gates further 
below too. So from what I've seen from the clock diagrams, there is nothing 
that would claim these clocks directly, and it wouldn't also make any sense to 
let them get disabled as there will always be something using them (for 
example the dram-controller).


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Mike Turquette]

Quoting Heiko St?bner (2014-07-31 16:29:34)
> Hi Mike,
> 
> Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> > Quoting Heiko Stuebner (2014-07-29 12:12:05)
> > 
> > > The clock-tree contains clocks that should never get disabled
> > > automatically. One example are the base ACLKs, the base supplies for all
> > > peripherals.
> > > 
> > > Therefore add a structure similar to the sunxi clock-tree to protect these
> > > special clocks from being disabled.
> > > 
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > > 
> > >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> > >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> > >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> > >  drivers/clk/rockchip/clk.h        |  1 +
> > >  4 files changed, 28 insertions(+)
> > > 
> > > diff --git a/drivers/clk/rockchip/clk-rk3188.c
> > > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> > > --- a/drivers/clk/rockchip/clk-rk3188.c
> > > +++ b/drivers/clk/rockchip/clk-rk3188.c
> > > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> > > rk3188_clk_branches[] __initdata = {> 
> > >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8),
> > >         13, GFLAGS),>  
> > >  };
> > > 
> > > +static const char *rk3188_critical_clocks[] __initconst = {
> > > +       "aclk_cpu",
> > > +       "aclk_peri",
> > 
> > I'm not against the idea of critical clocks, but I want to verify that
> > there is no other driver out there that is a better fit for claiming
> > these clks via clk_get and enabling them the normal way via clk_enable?
> 
> In the clock hierarchy of Rockchip SoCs, both aclks listed here, are sources 
> for pclk and hclk, as well as sourcing some other peripheral gates further 
> below too. So from what I've seen from the clock diagrams, there is nothing 
> that would claim these clocks directly, and it wouldn't also make any sense to 
> let them get disabled as there will always be something using them (for 
> example the dram-controller).

Sounds good. Just out of curiosity, under what circumstances would you
want to gate them? Is there a use case for it?

Regards,
Mike

> 
> 
> Heiko

Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Mike Turquette]

Quoting Heiko Stübner (2014-07-31 16:29:34)
> Hi Mike,
> 
> Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> > Quoting Heiko Stuebner (2014-07-29 12:12:05)
> > 
> > > The clock-tree contains clocks that should never get disabled
> > > automatically. One example are the base ACLKs, the base supplies for all
> > > peripherals.
> > > 
> > > Therefore add a structure similar to the sunxi clock-tree to protect these
> > > special clocks from being disabled.
> > > 
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > > 
> > >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> > >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> > >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> > >  drivers/clk/rockchip/clk.h        |  1 +
> > >  4 files changed, 28 insertions(+)
> > > 
> > > diff --git a/drivers/clk/rockchip/clk-rk3188.c
> > > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> > > --- a/drivers/clk/rockchip/clk-rk3188.c
> > > +++ b/drivers/clk/rockchip/clk-rk3188.c
> > > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> > > rk3188_clk_branches[] __initdata = {> 
> > >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8),
> > >         13, GFLAGS),>  
> > >  };
> > > 
> > > +static const char *rk3188_critical_clocks[] __initconst = {
> > > +       "aclk_cpu",
> > > +       "aclk_peri",
> > 
> > I'm not against the idea of critical clocks, but I want to verify that
> > there is no other driver out there that is a better fit for claiming
> > these clks via clk_get and enabling them the normal way via clk_enable?
> 
> In the clock hierarchy of Rockchip SoCs, both aclks listed here, are sources 
> for pclk and hclk, as well as sourcing some other peripheral gates further 
> below too. So from what I've seen from the clock diagrams, there is nothing 
> that would claim these clocks directly, and it wouldn't also make any sense to 
> let them get disabled as there will always be something using them (for 
> example the dram-controller).

Sounds good. Just out of curiosity, under what circumstances would you
want to gate them? Is there a use case for it?

Regards,
Mike

> 
> 
> Heiko

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

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stübner]

Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
> Quoting Heiko St?bner (2014-07-31 16:29:34)
> 
> > Hi Mike,
> > 
> > Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> > > Quoting Heiko Stuebner (2014-07-29 12:12:05)
> > > 
> > > > The clock-tree contains clocks that should never get disabled
> > > > automatically. One example are the base ACLKs, the base supplies for
> > > > all
> > > > peripherals.
> > > > 
> > > > Therefore add a structure similar to the sunxi clock-tree to protect
> > > > these
> > > > special clocks from being disabled.
> > > > 
> > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > > ---
> > > > 
> > > >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> > > >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> > > >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> > > >  drivers/clk/rockchip/clk.h        |  1 +
> > > >  4 files changed, 28 insertions(+)
> > > > 
> > > > diff --git a/drivers/clk/rockchip/clk-rk3188.c
> > > > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> > > > --- a/drivers/clk/rockchip/clk-rk3188.c
> > > > +++ b/drivers/clk/rockchip/clk-rk3188.c
> > > > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> > > > rk3188_clk_branches[] __initdata = {>
> > > > 
> > > >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
> > > >         RK2928_CLKGATE_CON(8),
> > > >         13, GFLAGS),>
> > > >  
> > > >  };
> > > > 
> > > > +static const char *rk3188_critical_clocks[] __initconst = {
> > > > +       "aclk_cpu",
> > > > +       "aclk_peri",
> > > 
> > > I'm not against the idea of critical clocks, but I want to verify that
> > > there is no other driver out there that is a better fit for claiming
> > > these clks via clk_get and enabling them the normal way via clk_enable?
> > 
> > In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
> > sources for pclk and hclk, as well as sourcing some other peripheral
> > gates further below too. So from what I've seen from the clock diagrams,
> > there is nothing that would claim these clocks directly, and it wouldn't
> > also make any sense to let them get disabled as there will always be
> > something using them (for example the dram-controller).
> 
> Sounds good. Just out of curiosity, under what circumstances would you
> want to gate them? Is there a use case for it?

hmm, I don't see a use-case for gating these at runtime right now, simply 
because there should be a user for them all the time. (both aclks combined 
have at least 68 consumers on the rk3288 and a similar number on the previous 
socs)

The only thing I could think of would be something suspend related - which we 
don't have yet. But then this would probably happen in the clock controller 
itself anyway in some late suspend-related action, so it could take into 
account them being defined as critical clocks.


Heiko

Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stübner]

Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
> Quoting Heiko Stübner (2014-07-31 16:29:34)
> 
> > Hi Mike,
> > 
> > Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> > > Quoting Heiko Stuebner (2014-07-29 12:12:05)
> > > 
> > > > The clock-tree contains clocks that should never get disabled
> > > > automatically. One example are the base ACLKs, the base supplies for
> > > > all
> > > > peripherals.
> > > > 
> > > > Therefore add a structure similar to the sunxi clock-tree to protect
> > > > these
> > > > special clocks from being disabled.
> > > > 
> > > > Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> > > > ---
> > > > 
> > > >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> > > >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> > > >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> > > >  drivers/clk/rockchip/clk.h        |  1 +
> > > >  4 files changed, 28 insertions(+)
> > > > 
> > > > diff --git a/drivers/clk/rockchip/clk-rk3188.c
> > > > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> > > > --- a/drivers/clk/rockchip/clk-rk3188.c
> > > > +++ b/drivers/clk/rockchip/clk-rk3188.c
> > > > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> > > > rk3188_clk_branches[] __initdata = {>
> > > > 
> > > >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
> > > >         RK2928_CLKGATE_CON(8),
> > > >         13, GFLAGS),>
> > > >  
> > > >  };
> > > > 
> > > > +static const char *rk3188_critical_clocks[] __initconst = {
> > > > +       "aclk_cpu",
> > > > +       "aclk_peri",
> > > 
> > > I'm not against the idea of critical clocks, but I want to verify that
> > > there is no other driver out there that is a better fit for claiming
> > > these clks via clk_get and enabling them the normal way via clk_enable?
> > 
> > In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
> > sources for pclk and hclk, as well as sourcing some other peripheral
> > gates further below too. So from what I've seen from the clock diagrams,
> > there is nothing that would claim these clocks directly, and it wouldn't
> > also make any sense to let them get disabled as there will always be
> > something using them (for example the dram-controller).
> 
> Sounds good. Just out of curiosity, under what circumstances would you
> want to gate them? Is there a use case for it?

hmm, I don't see a use-case for gating these at runtime right now, simply 
because there should be a user for them all the time. (both aclks combined 
have at least 68 consumers on the rk3288 and a similar number on the previous 
socs)

The only thing I could think of would be something suspend related - which we 
don't have yet. But then this would probably happen in the clock controller 
itself anyway in some late suspend-related action, so it could take into 
account them being defined as critical clocks.


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Doug Anderson]

Heiko,

On Fri, Aug 1, 2014 at 1:15 AM, Heiko St?bner <heiko@sntech.de> wrote:
> Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
>> Quoting Heiko St?bner (2014-07-31 16:29:34)
>>
>> > Hi Mike,
>> >
>> > Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
>> > > Quoting Heiko Stuebner (2014-07-29 12:12:05)
>> > >
>> > > > The clock-tree contains clocks that should never get disabled
>> > > > automatically. One example are the base ACLKs, the base supplies for
>> > > > all
>> > > > peripherals.
>> > > >
>> > > > Therefore add a structure similar to the sunxi clock-tree to protect
>> > > > these
>> > > > special clocks from being disabled.
>> > > >
>> > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>> > > > ---
>> > > >
>> > > >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
>> > > >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
>> > > >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
>> > > >  drivers/clk/rockchip/clk.h        |  1 +
>> > > >  4 files changed, 28 insertions(+)
>> > > >
>> > > > diff --git a/drivers/clk/rockchip/clk-rk3188.c
>> > > > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
>> > > > --- a/drivers/clk/rockchip/clk-rk3188.c
>> > > > +++ b/drivers/clk/rockchip/clk-rk3188.c
>> > > > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
>> > > > rk3188_clk_branches[] __initdata = {>
>> > > >
>> > > >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
>> > > >         RK2928_CLKGATE_CON(8),
>> > > >         13, GFLAGS),>
>> > > >
>> > > >  };
>> > > >
>> > > > +static const char *rk3188_critical_clocks[] __initconst = {
>> > > > +       "aclk_cpu",
>> > > > +       "aclk_peri",
>> > >
>> > > I'm not against the idea of critical clocks, but I want to verify that
>> > > there is no other driver out there that is a better fit for claiming
>> > > these clks via clk_get and enabling them the normal way via clk_enable?
>> >
>> > In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
>> > sources for pclk and hclk, as well as sourcing some other peripheral
>> > gates further below too. So from what I've seen from the clock diagrams,
>> > there is nothing that would claim these clocks directly, and it wouldn't
>> > also make any sense to let them get disabled as there will always be
>> > something using them (for example the dram-controller).
>>
>> Sounds good. Just out of curiosity, under what circumstances would you
>> want to gate them? Is there a use case for it?
>
> hmm, I don't see a use-case for gating these at runtime right now, simply
> because there should be a user for them all the time. (both aclks combined
> have at least 68 consumers on the rk3288 and a similar number on the previous
> socs)
>
> The only thing I could think of would be something suspend related - which we
> don't have yet. But then this would probably happen in the clock controller
> itself anyway in some late suspend-related action, so it could take into
> account them being defined as critical clocks.

I know Rockchip has some funky stuff planned for memory scaling too.
Perhaps Kever can comment whether these two clocks might need to be
disabled in that case?

In any case, this patch fixes a hang at boot when using the PWM driver
that just landed, so:

Tested-by: Doug Anderson <dianders@chromium.org>

Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Doug Anderson]

Heiko,

On Fri, Aug 1, 2014 at 1:15 AM, Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
>> Quoting Heiko Stübner (2014-07-31 16:29:34)
>>
>> > Hi Mike,
>> >
>> > Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
>> > > Quoting Heiko Stuebner (2014-07-29 12:12:05)
>> > >
>> > > > The clock-tree contains clocks that should never get disabled
>> > > > automatically. One example are the base ACLKs, the base supplies for
>> > > > all
>> > > > peripherals.
>> > > >
>> > > > Therefore add a structure similar to the sunxi clock-tree to protect
>> > > > these
>> > > > special clocks from being disabled.
>> > > >
>> > > > Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
>> > > > ---
>> > > >
>> > > >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
>> > > >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
>> > > >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
>> > > >  drivers/clk/rockchip/clk.h        |  1 +
>> > > >  4 files changed, 28 insertions(+)
>> > > >
>> > > > diff --git a/drivers/clk/rockchip/clk-rk3188.c
>> > > > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
>> > > > --- a/drivers/clk/rockchip/clk-rk3188.c
>> > > > +++ b/drivers/clk/rockchip/clk-rk3188.c
>> > > > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
>> > > > rk3188_clk_branches[] __initdata = {>
>> > > >
>> > > >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
>> > > >         RK2928_CLKGATE_CON(8),
>> > > >         13, GFLAGS),>
>> > > >
>> > > >  };
>> > > >
>> > > > +static const char *rk3188_critical_clocks[] __initconst = {
>> > > > +       "aclk_cpu",
>> > > > +       "aclk_peri",
>> > >
>> > > I'm not against the idea of critical clocks, but I want to verify that
>> > > there is no other driver out there that is a better fit for claiming
>> > > these clks via clk_get and enabling them the normal way via clk_enable?
>> >
>> > In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
>> > sources for pclk and hclk, as well as sourcing some other peripheral
>> > gates further below too. So from what I've seen from the clock diagrams,
>> > there is nothing that would claim these clocks directly, and it wouldn't
>> > also make any sense to let them get disabled as there will always be
>> > something using them (for example the dram-controller).
>>
>> Sounds good. Just out of curiosity, under what circumstances would you
>> want to gate them? Is there a use case for it?
>
> hmm, I don't see a use-case for gating these at runtime right now, simply
> because there should be a user for them all the time. (both aclks combined
> have at least 68 consumers on the rk3288 and a similar number on the previous
> socs)
>
> The only thing I could think of would be something suspend related - which we
> don't have yet. But then this would probably happen in the clock controller
> itself anyway in some late suspend-related action, so it could take into
> account them being defined as critical clocks.

I know Rockchip has some funky stuff planned for memory scaling too.
Perhaps Kever can comment whether these two clocks might need to be
disabled in that case?

In any case, this patch fixes a hang at boot when using the PWM driver
that just landed, so:

Tested-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stübner]

Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
> Heiko,
> 
> On Fri, Aug 1, 2014 at 1:15 AM, Heiko St?bner <heiko@sntech.de> wrote:
> > Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
> >> Quoting Heiko St?bner (2014-07-31 16:29:34)
> >> 
> >> > Hi Mike,
> >> > 
> >> > Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> >> > > Quoting Heiko Stuebner (2014-07-29 12:12:05)
> >> > > 
> >> > > > The clock-tree contains clocks that should never get disabled
> >> > > > automatically. One example are the base ACLKs, the base supplies
> >> > > > for
> >> > > > all
> >> > > > peripherals.
> >> > > > 
> >> > > > Therefore add a structure similar to the sunxi clock-tree to
> >> > > > protect
> >> > > > these
> >> > > > special clocks from being disabled.
> >> > > > 
> >> > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >> > > > ---
> >> > > > 
> >> > > >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> >> > > >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> >> > > >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> >> > > >  drivers/clk/rockchip/clk.h        |  1 +
> >> > > >  4 files changed, 28 insertions(+)
> >> > > > 
> >> > > > diff --git a/drivers/clk/rockchip/clk-rk3188.c
> >> > > > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> >> > > > --- a/drivers/clk/rockchip/clk-rk3188.c
> >> > > > +++ b/drivers/clk/rockchip/clk-rk3188.c
> >> > > > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> >> > > > rk3188_clk_branches[] __initdata = {>
> >> > > > 
> >> > > >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
> >> > > >         RK2928_CLKGATE_CON(8),
> >> > > >         13, GFLAGS),>
> >> > > >  
> >> > > >  };
> >> > > > 
> >> > > > +static const char *rk3188_critical_clocks[] __initconst = {
> >> > > > +       "aclk_cpu",
> >> > > > +       "aclk_peri",
> >> > > 
> >> > > I'm not against the idea of critical clocks, but I want to verify
> >> > > that
> >> > > there is no other driver out there that is a better fit for claiming
> >> > > these clks via clk_get and enabling them the normal way via
> >> > > clk_enable?
> >> > 
> >> > In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
> >> > sources for pclk and hclk, as well as sourcing some other peripheral
> >> > gates further below too. So from what I've seen from the clock
> >> > diagrams,
> >> > there is nothing that would claim these clocks directly, and it
> >> > wouldn't
> >> > also make any sense to let them get disabled as there will always be
> >> > something using them (for example the dram-controller).
> >> 
> >> Sounds good. Just out of curiosity, under what circumstances would you
> >> want to gate them? Is there a use case for it?
> > 
> > hmm, I don't see a use-case for gating these at runtime right now, simply
> > because there should be a user for them all the time. (both aclks combined
> > have at least 68 consumers on the rk3288 and a similar number on the
> > previous socs)
> > 
> > The only thing I could think of would be something suspend related - which
> > we don't have yet. But then this would probably happen in the clock
> > controller itself anyway in some late suspend-related action, so it could
> > take into account them being defined as critical clocks.
> 
> I know Rockchip has some funky stuff planned for memory scaling too.
> Perhaps Kever can comment whether these two clocks might need to be
> disabled in that case?

hmm looking at the core clock tree, I wouldn't think so.

The only intersection between the ddr-clk, aclk_cpu and aclk_peri is the gpll 
which can be a source to both. But the ddr-clk is mainly sourced from the dpll 
anyway.

In any case, turning off aclk_cpu/aclk_peri in this scenario wouldn't normally 
be possible anyway, as most of the time some pclk_* would be active anyway.


> In any case, this patch fixes a hang at boot when using the PWM driver
> that just landed, so:
> 
> Tested-by: Doug Anderson <dianders@chromium.org>

thanks


Heiko

Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stübner]

Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
> Heiko,
> 
> On Fri, Aug 1, 2014 at 1:15 AM, Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> > Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
> >> Quoting Heiko Stübner (2014-07-31 16:29:34)
> >> 
> >> > Hi Mike,
> >> > 
> >> > Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> >> > > Quoting Heiko Stuebner (2014-07-29 12:12:05)
> >> > > 
> >> > > > The clock-tree contains clocks that should never get disabled
> >> > > > automatically. One example are the base ACLKs, the base supplies
> >> > > > for
> >> > > > all
> >> > > > peripherals.
> >> > > > 
> >> > > > Therefore add a structure similar to the sunxi clock-tree to
> >> > > > protect
> >> > > > these
> >> > > > special clocks from being disabled.
> >> > > > 
> >> > > > Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> >> > > > ---
> >> > > > 
> >> > > >  drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> >> > > >  drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> >> > > >  drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> >> > > >  drivers/clk/rockchip/clk.h        |  1 +
> >> > > >  4 files changed, 28 insertions(+)
> >> > > > 
> >> > > > diff --git a/drivers/clk/rockchip/clk-rk3188.c
> >> > > > b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> >> > > > --- a/drivers/clk/rockchip/clk-rk3188.c
> >> > > > +++ b/drivers/clk/rockchip/clk-rk3188.c
> >> > > > @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> >> > > > rk3188_clk_branches[] __initdata = {>
> >> > > > 
> >> > > >         GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
> >> > > >         RK2928_CLKGATE_CON(8),
> >> > > >         13, GFLAGS),>
> >> > > >  
> >> > > >  };
> >> > > > 
> >> > > > +static const char *rk3188_critical_clocks[] __initconst = {
> >> > > > +       "aclk_cpu",
> >> > > > +       "aclk_peri",
> >> > > 
> >> > > I'm not against the idea of critical clocks, but I want to verify
> >> > > that
> >> > > there is no other driver out there that is a better fit for claiming
> >> > > these clks via clk_get and enabling them the normal way via
> >> > > clk_enable?
> >> > 
> >> > In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
> >> > sources for pclk and hclk, as well as sourcing some other peripheral
> >> > gates further below too. So from what I've seen from the clock
> >> > diagrams,
> >> > there is nothing that would claim these clocks directly, and it
> >> > wouldn't
> >> > also make any sense to let them get disabled as there will always be
> >> > something using them (for example the dram-controller).
> >> 
> >> Sounds good. Just out of curiosity, under what circumstances would you
> >> want to gate them? Is there a use case for it?
> > 
> > hmm, I don't see a use-case for gating these at runtime right now, simply
> > because there should be a user for them all the time. (both aclks combined
> > have at least 68 consumers on the rk3288 and a similar number on the
> > previous socs)
> > 
> > The only thing I could think of would be something suspend related - which
> > we don't have yet. But then this would probably happen in the clock
> > controller itself anyway in some late suspend-related action, so it could
> > take into account them being defined as critical clocks.
> 
> I know Rockchip has some funky stuff planned for memory scaling too.
> Perhaps Kever can comment whether these two clocks might need to be
> disabled in that case?

hmm looking at the core clock tree, I wouldn't think so.

The only intersection between the ddr-clk, aclk_cpu and aclk_peri is the gpll 
which can be a source to both. But the ddr-clk is mainly sourced from the dpll 
anyway.

In any case, turning off aclk_cpu/aclk_peri in this scenario wouldn't normally 
be possible anyway, as most of the time some pclk_* would be active anyway.


> In any case, this patch fixes a hang at boot when using the PWM driver
> that just landed, so:
> 
> Tested-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

thanks


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Kever Yang]

On 08/09/2014 06:20 AM, Heiko St?bner wrote:
> Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
>> Heiko,
>>
>> On Fri, Aug 1, 2014 at 1:15 AM, Heiko St?bner <heiko@sntech.de> wrote:
>>> Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
>>>> Quoting Heiko St?bner (2014-07-31 16:29:34)
>>>>
>>>>> Hi Mike,
>>>>>
>>>>> Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
>>>>>> Quoting Heiko Stuebner (2014-07-29 12:12:05)
>>>>>>
>>>>>>> The clock-tree contains clocks that should never get disabled
>>>>>>> automatically. One example are the base ACLKs, the base supplies
>>>>>>> for
>>>>>>> all
>>>>>>> peripherals.
>>>>>>>
>>>>>>> Therefore add a structure similar to the sunxi clock-tree to
>>>>>>> protect
>>>>>>> these
>>>>>>> special clocks from being disabled.
>>>>>>>
>>>>>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>>>>>> ---
>>>>>>>
>>>>>>>   drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
>>>>>>>   drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
>>>>>>>   drivers/clk/rockchip/clk.c        | 13 +++++++++++++
>>>>>>>   drivers/clk/rockchip/clk.h        |  1 +
>>>>>>>   4 files changed, 28 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/clk/rockchip/clk-rk3188.c
>>>>>>> b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
>>>>>>> --- a/drivers/clk/rockchip/clk-rk3188.c
>>>>>>> +++ b/drivers/clk/rockchip/clk-rk3188.c
>>>>>>> @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
>>>>>>> rk3188_clk_branches[] __initdata = {>
>>>>>>>
>>>>>>>          GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
>>>>>>>          RK2928_CLKGATE_CON(8),
>>>>>>>          13, GFLAGS),>
>>>>>>>   
>>>>>>>   };
>>>>>>>
>>>>>>> +static const char *rk3188_critical_clocks[] __initconst = {
>>>>>>> +       "aclk_cpu",
>>>>>>> +       "aclk_peri",
>>>>>> I'm not against the idea of critical clocks, but I want to verify
>>>>>> that
>>>>>> there is no other driver out there that is a better fit for claiming
>>>>>> these clks via clk_get and enabling them the normal way via
>>>>>> clk_enable?
>>>>> In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
>>>>> sources for pclk and hclk, as well as sourcing some other peripheral
>>>>> gates further below too. So from what I've seen from the clock
>>>>> diagrams,
>>>>> there is nothing that would claim these clocks directly, and it
>>>>> wouldn't
>>>>> also make any sense to let them get disabled as there will always be
>>>>> something using them (for example the dram-controller).
>>>> Sounds good. Just out of curiosity, under what circumstances would you
>>>> want to gate them? Is there a use case for it?
>>> hmm, I don't see a use-case for gating these at runtime right now, simply
>>> because there should be a user for them all the time. (both aclks combined
>>> have at least 68 consumers on the rk3288 and a similar number on the
>>> previous socs)
>>>
>>> The only thing I could think of would be something suspend related - which
>>> we don't have yet. But then this would probably happen in the clock
>>> controller itself anyway in some late suspend-related action, so it could
>>> take into account them being defined as critical clocks.
>> I know Rockchip has some funky stuff planned for memory scaling too.
>> Perhaps Kever can comment whether these two clocks might need to be
>> disabled in that case?
> hmm looking at the core clock tree, I wouldn't think so.
>
> The only intersection between the ddr-clk, aclk_cpu and aclk_peri is the gpll
> which can be a source to both. But the ddr-clk is mainly sourced from the dpll
> anyway.
>
> In any case, turning off aclk_cpu/aclk_peri in this scenario wouldn't normally
> be possible anyway, as most of the time some pclk_* would be active anyway.
Basically, aclk_cpu/aclk_peri have very little chance to be gated during 
run-time,
but both of then may be gated when system enter suspend mode.

For aclk_cpu, this clock supplies most of clocks in pd_bus actually, 
some clocks not listed
as a module clock will be needed, like cpu I/D bus fetch 
instruction/data from dram via
bus based on aclk_cpu. For this situation, can we use a dummy clock to 
hold the
aclk_cpu not to be gated at run-time?

For aclk_peri, this clock is able to be gated run-time in theory, 
although it's no use
in actual system, because we have many devices on this clock and at most 
of the time
some of then would be active just as you have mentioned.

The system suspend is another scenario, and we tend to gate both of the 
clock if possible,
can we do that if this patch is applied?

-Kever
>
>
>> In any case, this patch fixes a hang at boot when using the PWM driver
>> that just landed, so:
>>
>> Tested-by: Doug Anderson <dianders@chromium.org>
> thanks
>
>
> Heiko
>
>
>

Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Kever Yang]

On 08/09/2014 06:20 AM, Heiko Stübner wrote:
> Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
>> Heiko,
>>
>> On Fri, Aug 1, 2014 at 1:15 AM, Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
>>> Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
>>>> Quoting Heiko Stübner (2014-07-31 16:29:34)
>>>>
>>>>> Hi Mike,
>>>>>
>>>>> Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
>>>>>> Quoting Heiko Stuebner (2014-07-29 12:12:05)
>>>>>>
>>>>>>> The clock-tree contains clocks that should never get disabled
>>>>>>> automatically. One example are the base ACLKs, the base supplies
>>>>>>> for
>>>>>>> all
>>>>>>> peripherals.
>>>>>>>
>>>>>>> Therefore add a structure similar to the sunxi clock-tree to
>>>>>>> protect
>>>>>>> these
>>>>>>> special clocks from being disabled.
>>>>>>>
>>>>>>> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
>>>>>>> ---
>>>>>>>
>>>>>>>   drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
>>>>>>>   drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
>>>>>>>   drivers/clk/rockchip/clk.c        | 13 +++++++++++++
>>>>>>>   drivers/clk/rockchip/clk.h        |  1 +
>>>>>>>   4 files changed, 28 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/clk/rockchip/clk-rk3188.c
>>>>>>> b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
>>>>>>> --- a/drivers/clk/rockchip/clk-rk3188.c
>>>>>>> +++ b/drivers/clk/rockchip/clk-rk3188.c
>>>>>>> @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
>>>>>>> rk3188_clk_branches[] __initdata = {>
>>>>>>>
>>>>>>>          GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
>>>>>>>          RK2928_CLKGATE_CON(8),
>>>>>>>          13, GFLAGS),>
>>>>>>>   
>>>>>>>   };
>>>>>>>
>>>>>>> +static const char *rk3188_critical_clocks[] __initconst = {
>>>>>>> +       "aclk_cpu",
>>>>>>> +       "aclk_peri",
>>>>>> I'm not against the idea of critical clocks, but I want to verify
>>>>>> that
>>>>>> there is no other driver out there that is a better fit for claiming
>>>>>> these clks via clk_get and enabling them the normal way via
>>>>>> clk_enable?
>>>>> In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
>>>>> sources for pclk and hclk, as well as sourcing some other peripheral
>>>>> gates further below too. So from what I've seen from the clock
>>>>> diagrams,
>>>>> there is nothing that would claim these clocks directly, and it
>>>>> wouldn't
>>>>> also make any sense to let them get disabled as there will always be
>>>>> something using them (for example the dram-controller).
>>>> Sounds good. Just out of curiosity, under what circumstances would you
>>>> want to gate them? Is there a use case for it?
>>> hmm, I don't see a use-case for gating these at runtime right now, simply
>>> because there should be a user for them all the time. (both aclks combined
>>> have at least 68 consumers on the rk3288 and a similar number on the
>>> previous socs)
>>>
>>> The only thing I could think of would be something suspend related - which
>>> we don't have yet. But then this would probably happen in the clock
>>> controller itself anyway in some late suspend-related action, so it could
>>> take into account them being defined as critical clocks.
>> I know Rockchip has some funky stuff planned for memory scaling too.
>> Perhaps Kever can comment whether these two clocks might need to be
>> disabled in that case?
> hmm looking at the core clock tree, I wouldn't think so.
>
> The only intersection between the ddr-clk, aclk_cpu and aclk_peri is the gpll
> which can be a source to both. But the ddr-clk is mainly sourced from the dpll
> anyway.
>
> In any case, turning off aclk_cpu/aclk_peri in this scenario wouldn't normally
> be possible anyway, as most of the time some pclk_* would be active anyway.
Basically, aclk_cpu/aclk_peri have very little chance to be gated during 
run-time,
but both of then may be gated when system enter suspend mode.

For aclk_cpu, this clock supplies most of clocks in pd_bus actually, 
some clocks not listed
as a module clock will be needed, like cpu I/D bus fetch 
instruction/data from dram via
bus based on aclk_cpu. For this situation, can we use a dummy clock to 
hold the
aclk_cpu not to be gated at run-time?

For aclk_peri, this clock is able to be gated run-time in theory, 
although it's no use
in actual system, because we have many devices on this clock and at most 
of the time
some of then would be active just as you have mentioned.

The system suspend is another scenario, and we tend to gate both of the 
clock if possible,
can we do that if this patch is applied?

-Kever
>
>
>> In any case, this patch fixes a hang at boot when using the PWM driver
>> that just landed, so:
>>
>> Tested-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> thanks
>
>
> Heiko
>
>
>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stübner]

Hi Kever,

Am Montag, 11. August 2014, 18:03:33 schrieb Kever Yang:
> On 08/09/2014 06:20 AM, Heiko St?bner wrote:
> > Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
> >> Heiko,
> >> 
> >> On Fri, Aug 1, 2014 at 1:15 AM, Heiko St?bner <heiko@sntech.de> wrote:
> >>> Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
> >>>> Quoting Heiko St?bner (2014-07-31 16:29:34)
> >>>> 
> >>>>> Hi Mike,
> >>>>> 
> >>>>> Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> >>>>>> Quoting Heiko Stuebner (2014-07-29 12:12:05)
> >>>>>> 
> >>>>>>> The clock-tree contains clocks that should never get disabled
> >>>>>>> automatically. One example are the base ACLKs, the base supplies
> >>>>>>> for
> >>>>>>> all
> >>>>>>> peripherals.
> >>>>>>> 
> >>>>>>> Therefore add a structure similar to the sunxi clock-tree to
> >>>>>>> protect
> >>>>>>> these
> >>>>>>> special clocks from being disabled.
> >>>>>>> 
> >>>>>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >>>>>>> ---
> >>>>>>> 
> >>>>>>>   drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> >>>>>>>   drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> >>>>>>>   drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> >>>>>>>   drivers/clk/rockchip/clk.h        |  1 +
> >>>>>>>   4 files changed, 28 insertions(+)
> >>>>>>> 
> >>>>>>> diff --git a/drivers/clk/rockchip/clk-rk3188.c
> >>>>>>> b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> >>>>>>> --- a/drivers/clk/rockchip/clk-rk3188.c
> >>>>>>> +++ b/drivers/clk/rockchip/clk-rk3188.c
> >>>>>>> @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> >>>>>>> rk3188_clk_branches[] __initdata = {>
> >>>>>>> 
> >>>>>>>          GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
> >>>>>>>          RK2928_CLKGATE_CON(8),
> >>>>>>>          13, GFLAGS),>
> >>>>>>>   
> >>>>>>>   };
> >>>>>>> 
> >>>>>>> +static const char *rk3188_critical_clocks[] __initconst = {
> >>>>>>> +       "aclk_cpu",
> >>>>>>> +       "aclk_peri",
> >>>>>> 
> >>>>>> I'm not against the idea of critical clocks, but I want to verify
> >>>>>> that
> >>>>>> there is no other driver out there that is a better fit for claiming
> >>>>>> these clks via clk_get and enabling them the normal way via
> >>>>>> clk_enable?
> >>>>> 
> >>>>> In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
> >>>>> sources for pclk and hclk, as well as sourcing some other peripheral
> >>>>> gates further below too. So from what I've seen from the clock
> >>>>> diagrams,
> >>>>> there is nothing that would claim these clocks directly, and it
> >>>>> wouldn't
> >>>>> also make any sense to let them get disabled as there will always be
> >>>>> something using them (for example the dram-controller).
> >>>> 
> >>>> Sounds good. Just out of curiosity, under what circumstances would you
> >>>> want to gate them? Is there a use case for it?
> >>> 
> >>> hmm, I don't see a use-case for gating these at runtime right now,
> >>> simply
> >>> because there should be a user for them all the time. (both aclks
> >>> combined
> >>> have at least 68 consumers on the rk3288 and a similar number on the
> >>> previous socs)
> >>> 
> >>> The only thing I could think of would be something suspend related -
> >>> which
> >>> we don't have yet. But then this would probably happen in the clock
> >>> controller itself anyway in some late suspend-related action, so it
> >>> could
> >>> take into account them being defined as critical clocks.
> >> 
> >> I know Rockchip has some funky stuff planned for memory scaling too.
> >> Perhaps Kever can comment whether these two clocks might need to be
> >> disabled in that case?
> > 
> > hmm looking at the core clock tree, I wouldn't think so.
> > 
> > The only intersection between the ddr-clk, aclk_cpu and aclk_peri is the
> > gpll which can be a source to both. But the ddr-clk is mainly sourced
> > from the dpll anyway.
> > 
> > In any case, turning off aclk_cpu/aclk_peri in this scenario wouldn't
> > normally be possible anyway, as most of the time some pclk_* would be
> > active anyway.
> Basically, aclk_cpu/aclk_peri have very little chance to be gated during
> run-time,
> but both of then may be gated when system enter suspend mode.
> 
> For aclk_cpu, this clock supplies most of clocks in pd_bus actually,
> some clocks not listed
> as a module clock will be needed, like cpu I/D bus fetch
> instruction/data from dram via
> bus based on aclk_cpu. For this situation, can we use a dummy clock to
> hold the
> aclk_cpu not to be gated at run-time?
> 
> For aclk_peri, this clock is able to be gated run-time in theory,
> although it's no use
> in actual system, because we have many devices on this clock and at most
> of the time
> some of then would be active just as you have mentioned.
> 
> The system suspend is another scenario, and we tend to gate both of the
> clock if possible,
> can we do that if this patch is applied?

as we will need suspend operations in the clock driver anyway [likely 
something like the Samsung clk driver does], it shouldn't be a problem to lift 
the hold on the critical clocks when suspending.


Heiko

> 
> -Kever
> 
> >> In any case, this patch fixes a hang at boot when using the PWM driver
> >> that just landed, so:
> >> 
> >> Tested-by: Doug Anderson <dianders@chromium.org>
> > 
> > thanks
> > 
> > 
> > Heiko

Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Heiko Stübner]

Hi Kever,

Am Montag, 11. August 2014, 18:03:33 schrieb Kever Yang:
> On 08/09/2014 06:20 AM, Heiko Stübner wrote:
> > Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
> >> Heiko,
> >> 
> >> On Fri, Aug 1, 2014 at 1:15 AM, Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> >>> Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
> >>>> Quoting Heiko Stübner (2014-07-31 16:29:34)
> >>>> 
> >>>>> Hi Mike,
> >>>>> 
> >>>>> Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> >>>>>> Quoting Heiko Stuebner (2014-07-29 12:12:05)
> >>>>>> 
> >>>>>>> The clock-tree contains clocks that should never get disabled
> >>>>>>> automatically. One example are the base ACLKs, the base supplies
> >>>>>>> for
> >>>>>>> all
> >>>>>>> peripherals.
> >>>>>>> 
> >>>>>>> Therefore add a structure similar to the sunxi clock-tree to
> >>>>>>> protect
> >>>>>>> these
> >>>>>>> special clocks from being disabled.
> >>>>>>> 
> >>>>>>> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> >>>>>>> ---
> >>>>>>> 
> >>>>>>>   drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
> >>>>>>>   drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
> >>>>>>>   drivers/clk/rockchip/clk.c        | 13 +++++++++++++
> >>>>>>>   drivers/clk/rockchip/clk.h        |  1 +
> >>>>>>>   4 files changed, 28 insertions(+)
> >>>>>>> 
> >>>>>>> diff --git a/drivers/clk/rockchip/clk-rk3188.c
> >>>>>>> b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> >>>>>>> --- a/drivers/clk/rockchip/clk-rk3188.c
> >>>>>>> +++ b/drivers/clk/rockchip/clk-rk3188.c
> >>>>>>> @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> >>>>>>> rk3188_clk_branches[] __initdata = {>
> >>>>>>> 
> >>>>>>>          GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
> >>>>>>>          RK2928_CLKGATE_CON(8),
> >>>>>>>          13, GFLAGS),>
> >>>>>>>   
> >>>>>>>   };
> >>>>>>> 
> >>>>>>> +static const char *rk3188_critical_clocks[] __initconst = {
> >>>>>>> +       "aclk_cpu",
> >>>>>>> +       "aclk_peri",
> >>>>>> 
> >>>>>> I'm not against the idea of critical clocks, but I want to verify
> >>>>>> that
> >>>>>> there is no other driver out there that is a better fit for claiming
> >>>>>> these clks via clk_get and enabling them the normal way via
> >>>>>> clk_enable?
> >>>>> 
> >>>>> In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
> >>>>> sources for pclk and hclk, as well as sourcing some other peripheral
> >>>>> gates further below too. So from what I've seen from the clock
> >>>>> diagrams,
> >>>>> there is nothing that would claim these clocks directly, and it
> >>>>> wouldn't
> >>>>> also make any sense to let them get disabled as there will always be
> >>>>> something using them (for example the dram-controller).
> >>>> 
> >>>> Sounds good. Just out of curiosity, under what circumstances would you
> >>>> want to gate them? Is there a use case for it?
> >>> 
> >>> hmm, I don't see a use-case for gating these at runtime right now,
> >>> simply
> >>> because there should be a user for them all the time. (both aclks
> >>> combined
> >>> have at least 68 consumers on the rk3288 and a similar number on the
> >>> previous socs)
> >>> 
> >>> The only thing I could think of would be something suspend related -
> >>> which
> >>> we don't have yet. But then this would probably happen in the clock
> >>> controller itself anyway in some late suspend-related action, so it
> >>> could
> >>> take into account them being defined as critical clocks.
> >> 
> >> I know Rockchip has some funky stuff planned for memory scaling too.
> >> Perhaps Kever can comment whether these two clocks might need to be
> >> disabled in that case?
> > 
> > hmm looking at the core clock tree, I wouldn't think so.
> > 
> > The only intersection between the ddr-clk, aclk_cpu and aclk_peri is the
> > gpll which can be a source to both. But the ddr-clk is mainly sourced
> > from the dpll anyway.
> > 
> > In any case, turning off aclk_cpu/aclk_peri in this scenario wouldn't
> > normally be possible anyway, as most of the time some pclk_* would be
> > active anyway.
> Basically, aclk_cpu/aclk_peri have very little chance to be gated during
> run-time,
> but both of then may be gated when system enter suspend mode.
> 
> For aclk_cpu, this clock supplies most of clocks in pd_bus actually,
> some clocks not listed
> as a module clock will be needed, like cpu I/D bus fetch
> instruction/data from dram via
> bus based on aclk_cpu. For this situation, can we use a dummy clock to
> hold the
> aclk_cpu not to be gated at run-time?
> 
> For aclk_peri, this clock is able to be gated run-time in theory,
> although it's no use
> in actual system, because we have many devices on this clock and at most
> of the time
> some of then would be active just as you have mentioned.
> 
> The system suspend is another scenario, and we tend to gate both of the
> clock if possible,
> can we do that if this patch is applied?

as we will need suspend operations in the clock driver anyway [likely 
something like the Samsung clk driver does], it shouldn't be a problem to lift 
the hold on the critical clocks when suspending.


Heiko

> 
> -Kever
> 
> >> In any case, this patch fixes a hang at boot when using the PWM driver
> >> that just landed, so:
> >> 
> >> Tested-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > 
> > thanks
> > 
> > 
> > Heiko

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Kever Yang]

Heiko,

On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> The clock-tree contains clocks that should never get disabled automatically.
> One example are the base ACLKs, the base supplies for all peripherals.
>
> Therefore add a structure similar to the sunxi clock-tree to protect these
> special clocks from being disabled.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
>   drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
>   drivers/clk/rockchip/clk.c        | 13 +++++++++++++
>   drivers/clk/rockchip/clk.h        |  1 +
>   4 files changed, 28 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
> index a83a6d8..5aef277 100644
> --- a/drivers/clk/rockchip/clk-rk3188.c
> +++ b/drivers/clk/rockchip/clk-rk3188.c
> @@ -599,6 +599,11 @@ static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = {
>   	GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),
>   };
>   
> +static const char *rk3188_critical_clocks[] __initconst = {
> +	"aclk_cpu",
> +	"aclk_peri",
> +};
> +
>   static void __init rk3188_common_clk_init(struct device_node *np)
>   {
>   	void __iomem *reg_base;
> @@ -628,6 +633,8 @@ static void __init rk3188_common_clk_init(struct device_node *np)
>   				   RK3188_GRF_SOC_STATUS);
>   	rockchip_clk_register_branches(common_clk_branches,
>   				  ARRAY_SIZE(common_clk_branches));
> +	rockchip_clk_protect_critical(rk3188_critical_clocks,
> +				      ARRAY_SIZE(rk3188_critical_clocks));
>   
>   	rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
>   				  ROCKCHIP_SOFTRST_HIWORD_MASK);
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index 0d8c6c5..6c6f954 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -680,6 +680,11 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
>   	GATE(0, "pclk_isp_in", "ext_isp", 0, RK3288_CLKGATE_CON(16), 3, GFLAGS),
>   };
>   
> +static const char *rk3288_critical_clocks[] __initconst = {
> +	"aclk_cpu",
> +	"aclk_peri",
> +};
> +
>   static void __init rk3288_clk_init(struct device_node *np)
>   {
>   	void __iomem *reg_base;
> @@ -710,6 +715,8 @@ static void __init rk3288_clk_init(struct device_node *np)
>   				   RK3288_GRF_SOC_STATUS);
>   	rockchip_clk_register_branches(rk3288_clk_branches,
>   				  ARRAY_SIZE(rk3288_clk_branches));
> +	rockchip_clk_protect_critical(rk3288_critical_clocks,
> +				      ARRAY_SIZE(rk3288_critical_clocks));
>   
>   	rockchip_register_softrst(np, 9, reg_base + RK3288_SOFTRST_CON(0),
>   				  ROCKCHIP_SOFTRST_HIWORD_MASK);
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 278cf9d..9189f1b 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -242,3 +242,16 @@ void __init rockchip_clk_register_branches(
>   		rockchip_clk_add_lookup(clk, list->id);
>   	}
>   }
> +
> +void __init rockchip_clk_protect_critical(const char *clocks[], int nclocks)
> +{
> +	int i;
> +
> +	/* Protect the clocks that needs to stay on */
> +	for (i = 0; i < nclocks; i++) {
> +		struct clk *clk = __clk_lookup(clocks[i]);
> +
> +		if (clk)
> +			clk_prepare_enable(clk);
> +	}
> +}
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index 887cbde..2b0bca1 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -329,6 +329,7 @@ void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list,
>   				    unsigned int nr_clk);
>   void rockchip_clk_register_plls(struct rockchip_pll_clock *pll_list,
>   				unsigned int nr_pll, int grf_lock_offset);
> +void rockchip_clk_protect_critical(const char *clocks[], int nclocks);
>   
>   #define ROCKCHIP_SOFTRST_HIWORD_MASK	BIT(0)
>   

Tested-by: Kever Yang <kever.yang@rock-chips.com>

Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled

[Kever Yang]

Heiko,

On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> The clock-tree contains clocks that should never get disabled automatically.
> One example are the base ACLKs, the base supplies for all peripherals.
>
> Therefore add a structure similar to the sunxi clock-tree to protect these
> special clocks from being disabled.
>
> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
>   drivers/clk/rockchip/clk-rk3188.c |  7 +++++++
>   drivers/clk/rockchip/clk-rk3288.c |  7 +++++++
>   drivers/clk/rockchip/clk.c        | 13 +++++++++++++
>   drivers/clk/rockchip/clk.h        |  1 +
>   4 files changed, 28 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
> index a83a6d8..5aef277 100644
> --- a/drivers/clk/rockchip/clk-rk3188.c
> +++ b/drivers/clk/rockchip/clk-rk3188.c
> @@ -599,6 +599,11 @@ static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = {
>   	GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),
>   };
>   
> +static const char *rk3188_critical_clocks[] __initconst = {
> +	"aclk_cpu",
> +	"aclk_peri",
> +};
> +
>   static void __init rk3188_common_clk_init(struct device_node *np)
>   {
>   	void __iomem *reg_base;
> @@ -628,6 +633,8 @@ static void __init rk3188_common_clk_init(struct device_node *np)
>   				   RK3188_GRF_SOC_STATUS);
>   	rockchip_clk_register_branches(common_clk_branches,
>   				  ARRAY_SIZE(common_clk_branches));
> +	rockchip_clk_protect_critical(rk3188_critical_clocks,
> +				      ARRAY_SIZE(rk3188_critical_clocks));
>   
>   	rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
>   				  ROCKCHIP_SOFTRST_HIWORD_MASK);
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index 0d8c6c5..6c6f954 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -680,6 +680,11 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
>   	GATE(0, "pclk_isp_in", "ext_isp", 0, RK3288_CLKGATE_CON(16), 3, GFLAGS),
>   };
>   
> +static const char *rk3288_critical_clocks[] __initconst = {
> +	"aclk_cpu",
> +	"aclk_peri",
> +};
> +
>   static void __init rk3288_clk_init(struct device_node *np)
>   {
>   	void __iomem *reg_base;
> @@ -710,6 +715,8 @@ static void __init rk3288_clk_init(struct device_node *np)
>   				   RK3288_GRF_SOC_STATUS);
>   	rockchip_clk_register_branches(rk3288_clk_branches,
>   				  ARRAY_SIZE(rk3288_clk_branches));
> +	rockchip_clk_protect_critical(rk3288_critical_clocks,
> +				      ARRAY_SIZE(rk3288_critical_clocks));
>   
>   	rockchip_register_softrst(np, 9, reg_base + RK3288_SOFTRST_CON(0),
>   				  ROCKCHIP_SOFTRST_HIWORD_MASK);
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 278cf9d..9189f1b 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -242,3 +242,16 @@ void __init rockchip_clk_register_branches(
>   		rockchip_clk_add_lookup(clk, list->id);
>   	}
>   }
> +
> +void __init rockchip_clk_protect_critical(const char *clocks[], int nclocks)
> +{
> +	int i;
> +
> +	/* Protect the clocks that needs to stay on */
> +	for (i = 0; i < nclocks; i++) {
> +		struct clk *clk = __clk_lookup(clocks[i]);
> +
> +		if (clk)
> +			clk_prepare_enable(clk);
> +	}
> +}
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index 887cbde..2b0bca1 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -329,6 +329,7 @@ void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list,
>   				    unsigned int nr_clk);
>   void rockchip_clk_register_plls(struct rockchip_pll_clock *pll_list,
>   				unsigned int nr_pll, int grf_lock_offset);
> +void rockchip_clk_protect_critical(const char *clocks[], int nclocks);
>   
>   #define ROCKCHIP_SOFTRST_HIWORD_MASK	BIT(0)
>   

Tested-by: Kever Yang <kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Heiko Stuebner]

This is needed to access the pl330 dma controllers on Rockchip SoCs.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/mach-rockchip/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index d168669..ac5803c 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -4,6 +4,7 @@ config ARCH_ROCKCHIP
 	select PINCTRL_ROCKCHIP
 	select ARCH_HAS_RESET_CONTROLLER
 	select ARCH_REQUIRE_GPIOLIB
+	select ARM_AMBA
 	select ARM_GIC
 	select CACHE_L2X0
 	select HAVE_ARM_ARCH_TIMER
-- 
2.0.1

[PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Heiko Stuebner]

This is needed to access the pl330 dma controllers on Rockchip SoCs.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 arch/arm/mach-rockchip/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index d168669..ac5803c 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -4,6 +4,7 @@ config ARCH_ROCKCHIP
 	select PINCTRL_ROCKCHIP
 	select ARCH_HAS_RESET_CONTROLLER
 	select ARCH_REQUIRE_GPIOLIB
+	select ARM_AMBA
 	select ARM_GIC
 	select CACHE_L2X0
 	select HAVE_ARM_ARCH_TIMER
-- 
2.0.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Kever Yang]

Heiko:

On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> This is needed to access the pl330 dma controllers on Rockchip SoCs.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   arch/arm/mach-rockchip/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index d168669..ac5803c 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -4,6 +4,7 @@ config ARCH_ROCKCHIP
>   	select PINCTRL_ROCKCHIP
>   	select ARCH_HAS_RESET_CONTROLLER
>   	select ARCH_REQUIRE_GPIOLIB
> +	select ARM_AMBA
These two config is also needed by pl330 dma driver. Can we add it here?
     DMADEVICES
     PL330_DMA
>   	select ARM_GIC
>   	select CACHE_L2X0
>   	select HAVE_ARM_ARCH_TIMER

Re: [PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Kever Yang]

Heiko:

On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> This is needed to access the pl330 dma controllers on Rockchip SoCs.
>
> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
>   arch/arm/mach-rockchip/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index d168669..ac5803c 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -4,6 +4,7 @@ config ARCH_ROCKCHIP
>   	select PINCTRL_ROCKCHIP
>   	select ARCH_HAS_RESET_CONTROLLER
>   	select ARCH_REQUIRE_GPIOLIB
> +	select ARM_AMBA
These two config is also needed by pl330 dma driver. Can we add it here?
     DMADEVICES
     PL330_DMA
>   	select ARM_GIC
>   	select CACHE_L2X0
>   	select HAVE_ARM_ARCH_TIMER


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Heiko Stübner]

Am Montag, 11. August 2014, 11:35:15 schrieb Kever Yang:
> Heiko:
> 
> On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> > This is needed to access the pl330 dma controllers on Rockchip SoCs.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > 
> >   arch/arm/mach-rockchip/Kconfig | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/mach-rockchip/Kconfig
> > b/arch/arm/mach-rockchip/Kconfig index d168669..ac5803c 100644
> > --- a/arch/arm/mach-rockchip/Kconfig
> > +++ b/arch/arm/mach-rockchip/Kconfig
> > @@ -4,6 +4,7 @@ config ARCH_ROCKCHIP
> > 
> >   	select PINCTRL_ROCKCHIP
> >   	select ARCH_HAS_RESET_CONTROLLER
> >   	select ARCH_REQUIRE_GPIOLIB
> > 
> > +	select ARM_AMBA
> 
> These two config is also needed by pl330 dma driver. Can we add it here?
>      DMADEVICES
>      PL330_DMA

I guess this should simply be done via the regular Kconfig (and _defconfig) and 
not be automatically selected. There may always be use-cases where people do 
not want to use dma operations and drivers needing them.


Heiko

Re: [PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Heiko Stübner]

Am Montag, 11. August 2014, 11:35:15 schrieb Kever Yang:
> Heiko:
> 
> On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> > This is needed to access the pl330 dma controllers on Rockchip SoCs.
> > 
> > Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> > ---
> > 
> >   arch/arm/mach-rockchip/Kconfig | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/mach-rockchip/Kconfig
> > b/arch/arm/mach-rockchip/Kconfig index d168669..ac5803c 100644
> > --- a/arch/arm/mach-rockchip/Kconfig
> > +++ b/arch/arm/mach-rockchip/Kconfig
> > @@ -4,6 +4,7 @@ config ARCH_ROCKCHIP
> > 
> >   	select PINCTRL_ROCKCHIP
> >   	select ARCH_HAS_RESET_CONTROLLER
> >   	select ARCH_REQUIRE_GPIOLIB
> > 
> > +	select ARM_AMBA
> 
> These two config is also needed by pl330 dma driver. Can we add it here?
>      DMADEVICES
>      PL330_DMA

I guess this should simply be done via the regular Kconfig (and _defconfig) and 
not be automatically selected. There may always be use-cases where people do 
not want to use dma operations and drivers needing them.


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Doug Anderson]

Heiko,

On Tue, Jul 29, 2014 at 12:12 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> This is needed to access the pl330 dma controllers on Rockchip SoCs.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  arch/arm/mach-rockchip/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Doug Anderson <dianders@chromium.org>

Re: [PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Doug Anderson]

Heiko,

On Tue, Jul 29, 2014 at 12:12 PM, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> This is needed to access the pl330 dma controllers on Rockchip SoCs.
>
> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
>  arch/arm/mach-rockchip/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Kever Yang]

On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> This is needed to access the pl330 dma controllers on Rockchip SoCs.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   arch/arm/mach-rockchip/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index d168669..ac5803c 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -4,6 +4,7 @@ config ARCH_ROCKCHIP
>   	select PINCTRL_ROCKCHIP
>   	select ARCH_HAS_RESET_CONTROLLER
>   	select ARCH_REQUIRE_GPIOLIB
> +	select ARM_AMBA
>   	select ARM_GIC
>   	select CACHE_L2X0
>   	select HAVE_ARM_ARCH_TIMER
Tested-by: Kever Yang <kever.yang@rock-chips.com>

Re: [PATCH 2/4] ARM: rockchip: enable the AMBA bus

[Kever Yang]

On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> This is needed to access the pl330 dma controllers on Rockchip SoCs.
>
> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
>   arch/arm/mach-rockchip/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index d168669..ac5803c 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -4,6 +4,7 @@ config ARCH_ROCKCHIP
>   	select PINCTRL_ROCKCHIP
>   	select ARCH_HAS_RESET_CONTROLLER
>   	select ARCH_REQUIRE_GPIOLIB
> +	select ARM_AMBA
>   	select ARM_GIC
>   	select CACHE_L2X0
>   	select HAVE_ARM_ARCH_TIMER
Tested-by: Kever Yang <kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Heiko Stuebner]

Add both the bus and peripheral pl330 dma controllers present in rk3288 socs.
The first dma controller can change between secure and non-secure mode and is
left by the bootloader in secure mode, which gets added here.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 3ef8951..3f39d26 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -62,6 +62,33 @@
 		};
 	};
 
+	amba {
+		compatible = "arm,amba-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		dmac1: dma at ffb20000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0xffb20000 0x4000>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			clocks = <&cru ACLK_DMAC1>;
+			clock-names = "apb_pclk";
+		};
+
+		dmac2: dma at ff250000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0xff250000 0x4000>;
+			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			clocks = <&cru ACLK_DMAC2>;
+			clock-names = "apb_pclk";
+		};
+	};
+
 	xin24m: oscillator {
 		compatible = "fixed-clock";
 		clock-frequency = <24000000>;
-- 
2.0.1

[PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Heiko Stuebner]

Add both the bus and peripheral pl330 dma controllers present in rk3288 socs.
The first dma controller can change between secure and non-secure mode and is
left by the bootloader in secure mode, which gets added here.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 3ef8951..3f39d26 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -62,6 +62,33 @@
 		};
 	};
 
+	amba {
+		compatible = "arm,amba-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		dmac1: dma@ffb20000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0xffb20000 0x4000>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			clocks = <&cru ACLK_DMAC1>;
+			clock-names = "apb_pclk";
+		};
+
+		dmac2: dma@ff250000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0xff250000 0x4000>;
+			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			clocks = <&cru ACLK_DMAC2>;
+			clock-names = "apb_pclk";
+		};
+	};
+
 	xin24m: oscillator {
 		compatible = "fixed-clock";
 		clock-frequency = <24000000>;
-- 
2.0.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Doug Anderson]

Heiko,

On Tue, Jul 29, 2014 at 12:12 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> Add both the bus and peripheral pl330 dma controllers present in rk3288 socs.
> The first dma controller can change between secure and non-secure mode and is
> left by the bootloader in secure mode, which gets added here.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

This is outside of my area of expertise, but comparing this to
bindings, TRM, and other platforms it looks reasonable to me.


> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index 3ef8951..3f39d26 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -62,6 +62,33 @@
>                 };
>         };
>
> +       amba {
> +               compatible = "arm,amba-bus";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +
> +               dmac1: dma at ffb20000 {

I'm curious: why did you choose "dmac1" for the alias here?  I would
have called it "dmac_peri", "pdma", or something similar as per the
address map in the TRM.  I found a single reference in the TRM to
"dmac2" but it wasn't immediately clear to me which DMA that was
referring to.  In a commit in a temporary tree I see that someone at
Rockchip called the DMA at ff250000 "pdma1".


> +                       compatible = "arm,pl330", "arm,primecell";
> +                       reg = <0xffb20000 0x4000>;
> +                       interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> +                       #dma-cells = <1>;
> +                       clocks = <&cru ACLK_DMAC1>;
> +                       clock-names = "apb_pclk";
> +               };
> +
> +               dmac2: dma at ff250000 {

nit: when we have easy addresses to compare to, we should probably
sort by address?


> +                       compatible = "arm,pl330", "arm,primecell";
> +                       reg = <0xff250000 0x4000>;
> +                       interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> +                       #dma-cells = <1>;
> +                       clocks = <&cru ACLK_DMAC2>;
> +                       clock-names = "apb_pclk";
> +               };
> +       };
> +
>         xin24m: oscillator {
>                 compatible = "fixed-clock";
>                 clock-frequency = <24000000>;
> --
> 2.0.1
>

Re: [PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Doug Anderson]

Heiko,

On Tue, Jul 29, 2014 at 12:12 PM, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> Add both the bus and peripheral pl330 dma controllers present in rk3288 socs.
> The first dma controller can change between secure and non-secure mode and is
> left by the bootloader in secure mode, which gets added here.
>
> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

This is outside of my area of expertise, but comparing this to
bindings, TRM, and other platforms it looks reasonable to me.


> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index 3ef8951..3f39d26 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -62,6 +62,33 @@
>                 };
>         };
>
> +       amba {
> +               compatible = "arm,amba-bus";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +
> +               dmac1: dma@ffb20000 {

I'm curious: why did you choose "dmac1" for the alias here?  I would
have called it "dmac_peri", "pdma", or something similar as per the
address map in the TRM.  I found a single reference in the TRM to
"dmac2" but it wasn't immediately clear to me which DMA that was
referring to.  In a commit in a temporary tree I see that someone at
Rockchip called the DMA at ff250000 "pdma1".


> +                       compatible = "arm,pl330", "arm,primecell";
> +                       reg = <0xffb20000 0x4000>;
> +                       interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> +                       #dma-cells = <1>;
> +                       clocks = <&cru ACLK_DMAC1>;
> +                       clock-names = "apb_pclk";
> +               };
> +
> +               dmac2: dma@ff250000 {

nit: when we have easy addresses to compare to, we should probably
sort by address?


> +                       compatible = "arm,pl330", "arm,primecell";
> +                       reg = <0xff250000 0x4000>;
> +                       interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> +                       #dma-cells = <1>;
> +                       clocks = <&cru ACLK_DMAC2>;
> +                       clock-names = "apb_pclk";
> +               };
> +       };
> +
>         xin24m: oscillator {
>                 compatible = "fixed-clock";
>                 clock-frequency = <24000000>;
> --
> 2.0.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Heiko Stübner]

Am Montag, 11. August 2014, 10:01:08 schrieb Doug Anderson:
> Heiko,
> 
> On Tue, Jul 29, 2014 at 12:12 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> > Add both the bus and peripheral pl330 dma controllers present in rk3288
> > socs. The first dma controller can change between secure and non-secure
> > mode and is left by the bootloader in secure mode, which gets added here.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > 
> >  arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> 
> This is outside of my area of expertise, but comparing this to
> bindings, TRM, and other platforms it looks reasonable to me.
> 
> > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> > index 3ef8951..3f39d26 100644
> > --- a/arch/arm/boot/dts/rk3288.dtsi
> > +++ b/arch/arm/boot/dts/rk3288.dtsi
> > @@ -62,6 +62,33 @@
> > 
> >                 };
> >         
> >         };
> > 
> > +       amba {
> > +               compatible = "arm,amba-bus";
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               ranges;
> > +
> > +               dmac1: dma at ffb20000 {
> 
> I'm curious: why did you choose "dmac1" for the alias here?  I would
> have called it "dmac_peri", "pdma", or something similar as per the
> address map in the TRM.  I found a single reference in the TRM to
> "dmac2" but it wasn't immediately clear to me which DMA that was
> referring to.  In a commit in a temporary tree I see that someone at
> Rockchip called the DMA at ff250000 "pdma1".

I think I just kept the alias similar to the clock-name. The naming of the 
dma-controllers seems to be inconsistent ... the clocks are named dmac1 and 
dmac2, while the interrupt table in the rk3188-TRM calls them dmac0 and dmac1. 
So I guess I just duplicated the naming for the rk3288.

I guess for the rk3288, the one at 0xffb20000 should be named something like 
dmac_bus and the other one dmac_peri, reflecting what they're called in the 
TRM, but I'm of course open for suggestions :-) .


Also for an upcoming v2, I've also changed the structure a bit, as the first 
dma-controller has both a secure and non-secure version of it.
So currently the rk3288.dtsi looks like [0]:

	amba {
		compatible = "arm,amba-bus";
		#address-cells = <1>;
		#size-cells = <1>;
		ranges;

		/* dma1 in secure state */
		dma-controller at ffb20000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xffb20000 0x4000>;
			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC1>;
			clock-names = "apb_pclk";
			status = "disabled";
		};

		/* dma1 in non-secure state */
		dma-controller at ffb60000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xffb60000 0x4000>;
			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC1>;
			clock-names = "apb_pclk";
			status = "disabled";
		};

		dmac2: dma-controller at ff250000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xff250000 0x4000>;
			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC2>;
			clock-names = "apb_pclk";
		};
	};

and the board is responsible for enabling the correct variant [1], as most 
likely the bootloader decides in which mode to start the dma controller:

	amba {
		/* dma1 in secure state */
		dmac1: dma-controller at ffb20000 {
			status = "okay";
		};
	};

This is based on some mailinglist discussion, I found at some point, about 
this but for the life of me am not able to find anymore. So of course feedback 
would appreciated there too.



[0] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288.dtsi
[1] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288-evb.dtsi

> 
> > +                       compatible = "arm,pl330", "arm,primecell";
> > +                       reg = <0xffb20000 0x4000>;
> > +                       interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> > +                       #dma-cells = <1>;
> > +                       clocks = <&cru ACLK_DMAC1>;
> > +                       clock-names = "apb_pclk";
> > +               };
> > +
> > +               dmac2: dma at ff250000 {
> 
> nit: when we have easy addresses to compare to, we should probably
> sort by address?

correct :-)

> 
> > +                       compatible = "arm,pl330", "arm,primecell";
> > +                       reg = <0xff250000 0x4000>;
> > +                       interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> > +                       #dma-cells = <1>;
> > +                       clocks = <&cru ACLK_DMAC2>;
> > +                       clock-names = "apb_pclk";
> > +               };
> > +       };
> > +
> > 
> >         xin24m: oscillator {
> >         
> >                 compatible = "fixed-clock";
> >                 clock-frequency = <24000000>;
> > 
> > --
> > 2.0.1

Re: [PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Heiko Stübner]

Am Montag, 11. August 2014, 10:01:08 schrieb Doug Anderson:
> Heiko,
> 
> On Tue, Jul 29, 2014 at 12:12 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> > Add both the bus and peripheral pl330 dma controllers present in rk3288
> > socs. The first dma controller can change between secure and non-secure
> > mode and is left by the bootloader in secure mode, which gets added here.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > 
> >  arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> 
> This is outside of my area of expertise, but comparing this to
> bindings, TRM, and other platforms it looks reasonable to me.
> 
> > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> > index 3ef8951..3f39d26 100644
> > --- a/arch/arm/boot/dts/rk3288.dtsi
> > +++ b/arch/arm/boot/dts/rk3288.dtsi
> > @@ -62,6 +62,33 @@
> > 
> >                 };
> >         
> >         };
> > 
> > +       amba {
> > +               compatible = "arm,amba-bus";
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               ranges;
> > +
> > +               dmac1: dma@ffb20000 {
> 
> I'm curious: why did you choose "dmac1" for the alias here?  I would
> have called it "dmac_peri", "pdma", or something similar as per the
> address map in the TRM.  I found a single reference in the TRM to
> "dmac2" but it wasn't immediately clear to me which DMA that was
> referring to.  In a commit in a temporary tree I see that someone at
> Rockchip called the DMA at ff250000 "pdma1".

I think I just kept the alias similar to the clock-name. The naming of the 
dma-controllers seems to be inconsistent ... the clocks are named dmac1 and 
dmac2, while the interrupt table in the rk3188-TRM calls them dmac0 and dmac1. 
So I guess I just duplicated the naming for the rk3288.

I guess for the rk3288, the one at 0xffb20000 should be named something like 
dmac_bus and the other one dmac_peri, reflecting what they're called in the 
TRM, but I'm of course open for suggestions :-) .


Also for an upcoming v2, I've also changed the structure a bit, as the first 
dma-controller has both a secure and non-secure version of it.
So currently the rk3288.dtsi looks like [0]:

	amba {
		compatible = "arm,amba-bus";
		#address-cells = <1>;
		#size-cells = <1>;
		ranges;

		/* dma1 in secure state */
		dma-controller@ffb20000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xffb20000 0x4000>;
			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC1>;
			clock-names = "apb_pclk";
			status = "disabled";
		};

		/* dma1 in non-secure state */
		dma-controller@ffb60000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xffb60000 0x4000>;
			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC1>;
			clock-names = "apb_pclk";
			status = "disabled";
		};

		dmac2: dma-controller@ff250000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xff250000 0x4000>;
			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC2>;
			clock-names = "apb_pclk";
		};
	};

and the board is responsible for enabling the correct variant [1], as most 
likely the bootloader decides in which mode to start the dma controller:

	amba {
		/* dma1 in secure state */
		dmac1: dma-controller@ffb20000 {
			status = "okay";
		};
	};

This is based on some mailinglist discussion, I found at some point, about 
this but for the life of me am not able to find anymore. So of course feedback 
would appreciated there too.



[0] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288.dtsi
[1] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288-evb.dtsi

> 
> > +                       compatible = "arm,pl330", "arm,primecell";
> > +                       reg = <0xffb20000 0x4000>;
> > +                       interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> > +                       #dma-cells = <1>;
> > +                       clocks = <&cru ACLK_DMAC1>;
> > +                       clock-names = "apb_pclk";
> > +               };
> > +
> > +               dmac2: dma@ff250000 {
> 
> nit: when we have easy addresses to compare to, we should probably
> sort by address?

correct :-)

> 
> > +                       compatible = "arm,pl330", "arm,primecell";
> > +                       reg = <0xff250000 0x4000>;
> > +                       interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> > +                       #dma-cells = <1>;
> > +                       clocks = <&cru ACLK_DMAC2>;
> > +                       clock-names = "apb_pclk";
> > +               };
> > +       };
> > +
> > 
> >         xin24m: oscillator {
> >         
> >                 compatible = "fixed-clock";
> >                 clock-frequency = <24000000>;
> > 
> > --
> > 2.0.1

[PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Andreas Färber]

Am 11.08.2014 19:01, schrieb Heiko St?bner:
> Also for an upcoming v2, I've also changed the structure a bit, as the first 
> dma-controller has both a secure and non-secure version of it.
> So currently the rk3288.dtsi looks like [0]:
> 
> 	amba {
> 		compatible = "arm,amba-bus";
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		ranges;
> 
> 		/* dma1 in secure state */
> 		dma-controller at ffb20000 {
> 			compatible = "arm,pl330", "arm,primecell";
> 			reg = <0xffb20000 0x4000>;
> 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> 				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> 			#dma-cells = <1>;
> 			clocks = <&cru ACLK_DMAC1>;
> 			clock-names = "apb_pclk";
> 			status = "disabled";
> 		};
> 
> 		/* dma1 in non-secure state */
> 		dma-controller at ffb60000 {
> 			compatible = "arm,pl330", "arm,primecell";
> 			reg = <0xffb60000 0x4000>;
> 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> 				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> 			#dma-cells = <1>;
> 			clocks = <&cru ACLK_DMAC1>;
> 			clock-names = "apb_pclk";
> 			status = "disabled";
> 		};
> 
> 		dmac2: dma-controller at ff250000 {
> 			compatible = "arm,pl330", "arm,primecell";
> 			reg = <0xff250000 0x4000>;
> 			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> 				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> 			#dma-cells = <1>;
> 			clocks = <&cru ACLK_DMAC2>;
> 			clock-names = "apb_pclk";
> 		};
> 	};
> 
> and the board is responsible for enabling the correct variant [1], as most 
> likely the bootloader decides in which mode to start the dma controller:
> 
> 	amba {
> 		/* dma1 in secure state */
> 		dmac1: dma-controller at ffb20000 {
> 			status = "okay";
> 		};
> 	};
> 
> This is based on some mailinglist discussion, I found at some point, about 
> this but for the life of me am not able to find anymore. So of course feedback 
> would appreciated there too.

I would suggest to give labels such as dmac1_s and dmac1_ns like you did
for dmac2 in the former snippet, so that you can override the status in
the latter without replicating the amba/dma-controller hierarchy.

However, for the Zynq SoC we chose to just model the secure DMAC though
[*]. I was told that either the bootloader or the user should change the
DT when running in such a non-secure scenario.

Cheers,
Andreas

[*] https://patchwork.kernel.org/patch/4620251/

> [0] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288.dtsi
> [1] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288-evb.dtsi
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg

Re: [PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Andreas Färber]

Am 11.08.2014 19:01, schrieb Heiko Stübner:
> Also for an upcoming v2, I've also changed the structure a bit, as the first 
> dma-controller has both a secure and non-secure version of it.
> So currently the rk3288.dtsi looks like [0]:
> 
> 	amba {
> 		compatible = "arm,amba-bus";
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		ranges;
> 
> 		/* dma1 in secure state */
> 		dma-controller@ffb20000 {
> 			compatible = "arm,pl330", "arm,primecell";
> 			reg = <0xffb20000 0x4000>;
> 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> 				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> 			#dma-cells = <1>;
> 			clocks = <&cru ACLK_DMAC1>;
> 			clock-names = "apb_pclk";
> 			status = "disabled";
> 		};
> 
> 		/* dma1 in non-secure state */
> 		dma-controller@ffb60000 {
> 			compatible = "arm,pl330", "arm,primecell";
> 			reg = <0xffb60000 0x4000>;
> 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> 				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> 			#dma-cells = <1>;
> 			clocks = <&cru ACLK_DMAC1>;
> 			clock-names = "apb_pclk";
> 			status = "disabled";
> 		};
> 
> 		dmac2: dma-controller@ff250000 {
> 			compatible = "arm,pl330", "arm,primecell";
> 			reg = <0xff250000 0x4000>;
> 			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> 				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> 			#dma-cells = <1>;
> 			clocks = <&cru ACLK_DMAC2>;
> 			clock-names = "apb_pclk";
> 		};
> 	};
> 
> and the board is responsible for enabling the correct variant [1], as most 
> likely the bootloader decides in which mode to start the dma controller:
> 
> 	amba {
> 		/* dma1 in secure state */
> 		dmac1: dma-controller@ffb20000 {
> 			status = "okay";
> 		};
> 	};
> 
> This is based on some mailinglist discussion, I found at some point, about 
> this but for the life of me am not able to find anymore. So of course feedback 
> would appreciated there too.

I would suggest to give labels such as dmac1_s and dmac1_ns like you did
for dmac2 in the former snippet, so that you can override the status in
the latter without replicating the amba/dma-controller hierarchy.

However, for the Zynq SoC we chose to just model the secure DMAC though
[*]. I was told that either the bootloader or the user should change the
DT when running in such a non-secure scenario.

Cheers,
Andreas

[*] https://patchwork.kernel.org/patch/4620251/

> [0] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288.dtsi
> [1] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288-evb.dtsi
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Heiko Stübner]

Am Montag, 11. August 2014, 19:37:29 schrieb Andreas F?rber:
> Am 11.08.2014 19:01, schrieb Heiko St?bner:
> > Also for an upcoming v2, I've also changed the structure a bit, as the
> > first dma-controller has both a secure and non-secure version of it.
> > 
> > So currently the rk3288.dtsi looks like [0]:
> > 	amba {
> > 	
> > 		compatible = "arm,amba-bus";
> > 		#address-cells = <1>;
> > 		#size-cells = <1>;
> > 		ranges;
> > 		
> > 		/* dma1 in secure state */
> > 		dma-controller at ffb20000 {
> > 		
> > 			compatible = "arm,pl330", "arm,primecell";
> > 			reg = <0xffb20000 0x4000>;
> > 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> > 			
> > 				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> > 			
> > 			#dma-cells = <1>;
> > 			clocks = <&cru ACLK_DMAC1>;
> > 			clock-names = "apb_pclk";
> > 			status = "disabled";
> > 		
> > 		};
> > 		
> > 		/* dma1 in non-secure state */
> > 		dma-controller at ffb60000 {
> > 		
> > 			compatible = "arm,pl330", "arm,primecell";
> > 			reg = <0xffb60000 0x4000>;
> > 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> > 			
> > 				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> > 			
> > 			#dma-cells = <1>;
> > 			clocks = <&cru ACLK_DMAC1>;
> > 			clock-names = "apb_pclk";
> > 			status = "disabled";
> > 		
> > 		};
> > 		
> > 		dmac2: dma-controller at ff250000 {
> > 		
> > 			compatible = "arm,pl330", "arm,primecell";
> > 			reg = <0xff250000 0x4000>;
> > 			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> > 			
> > 				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> > 			
> > 			#dma-cells = <1>;
> > 			clocks = <&cru ACLK_DMAC2>;
> > 			clock-names = "apb_pclk";
> > 		
> > 		};
> > 	
> > 	};
> > 
> > and the board is responsible for enabling the correct variant [1], as most
> > 
> > likely the bootloader decides in which mode to start the dma controller:
> > 	amba {
> > 	
> > 		/* dma1 in secure state */
> > 		dmac1: dma-controller at ffb20000 {
> > 		
> > 			status = "okay";
> > 		
> > 		};
> > 	
> > 	};
> > 
> > This is based on some mailinglist discussion, I found at some point, about
> > this but for the life of me am not able to find anymore. So of course
> > feedback would appreciated there too.
> 
> I would suggest to give labels such as dmac1_s and dmac1_ns like you did
> for dmac2 in the former snippet, so that you can override the status in
> the latter without replicating the amba/dma-controller hierarchy.

I was hoping to just keep it to one handle, so that dma consumers would just 
work and not have to care which dma controller variant was available :-) .


> However, for the Zynq SoC we chose to just model the secure DMAC though
> [*]. I was told that either the bootloader or the user should change the
> DT when running in such a non-secure scenario.

ok, this sounds interesting too, as I haven't seen a board that used the dmac 
in question in non-secure mode, yet.

So I guess one way to handle it could be to declare the non-secure dma, but 
set it to a disabled state and use the secure one as default for everything:

		dmac_bus_s: dma-controller at ffb20000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xffb20000 0x4000>;
			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC1>;
			clock-names = "apb_pclk";
		};

		dmac_bus_ns: dma-controller at ffb60000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xffb60000 0x4000>;
			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC1>;
			clock-names = "apb_pclk";
			status = "disabled";
		};

and a board wanting to use the non-secure variant would then be required to 
override the status and the dma channels in question (this dmac does mainly 
i2s and spdif):

&dmac_bus_s {
	status = "disabled";
}:

&dmac_bus_ns {
	status = "okay";
};

&i2s0 {
	dmas = <&dmac_bus_ns 6>, <&dmac_bus_ns 7>;
};


> [*] https://patchwork.kernel.org/patch/4620251/
> 
> > [0]
> > https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boo
> > t/dts/rk3288.dtsi [1]
> > https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boo
> > t/dts/rk3288-evb.dtsi
> [snip]

Re: [PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Heiko Stübner]

Am Montag, 11. August 2014, 19:37:29 schrieb Andreas Färber:
> Am 11.08.2014 19:01, schrieb Heiko Stübner:
> > Also for an upcoming v2, I've also changed the structure a bit, as the
> > first dma-controller has both a secure and non-secure version of it.
> > 
> > So currently the rk3288.dtsi looks like [0]:
> > 	amba {
> > 	
> > 		compatible = "arm,amba-bus";
> > 		#address-cells = <1>;
> > 		#size-cells = <1>;
> > 		ranges;
> > 		
> > 		/* dma1 in secure state */
> > 		dma-controller@ffb20000 {
> > 		
> > 			compatible = "arm,pl330", "arm,primecell";
> > 			reg = <0xffb20000 0x4000>;
> > 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> > 			
> > 				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> > 			
> > 			#dma-cells = <1>;
> > 			clocks = <&cru ACLK_DMAC1>;
> > 			clock-names = "apb_pclk";
> > 			status = "disabled";
> > 		
> > 		};
> > 		
> > 		/* dma1 in non-secure state */
> > 		dma-controller@ffb60000 {
> > 		
> > 			compatible = "arm,pl330", "arm,primecell";
> > 			reg = <0xffb60000 0x4000>;
> > 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> > 			
> > 				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> > 			
> > 			#dma-cells = <1>;
> > 			clocks = <&cru ACLK_DMAC1>;
> > 			clock-names = "apb_pclk";
> > 			status = "disabled";
> > 		
> > 		};
> > 		
> > 		dmac2: dma-controller@ff250000 {
> > 		
> > 			compatible = "arm,pl330", "arm,primecell";
> > 			reg = <0xff250000 0x4000>;
> > 			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> > 			
> > 				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> > 			
> > 			#dma-cells = <1>;
> > 			clocks = <&cru ACLK_DMAC2>;
> > 			clock-names = "apb_pclk";
> > 		
> > 		};
> > 	
> > 	};
> > 
> > and the board is responsible for enabling the correct variant [1], as most
> > 
> > likely the bootloader decides in which mode to start the dma controller:
> > 	amba {
> > 	
> > 		/* dma1 in secure state */
> > 		dmac1: dma-controller@ffb20000 {
> > 		
> > 			status = "okay";
> > 		
> > 		};
> > 	
> > 	};
> > 
> > This is based on some mailinglist discussion, I found at some point, about
> > this but for the life of me am not able to find anymore. So of course
> > feedback would appreciated there too.
> 
> I would suggest to give labels such as dmac1_s and dmac1_ns like you did
> for dmac2 in the former snippet, so that you can override the status in
> the latter without replicating the amba/dma-controller hierarchy.

I was hoping to just keep it to one handle, so that dma consumers would just 
work and not have to care which dma controller variant was available :-) .


> However, for the Zynq SoC we chose to just model the secure DMAC though
> [*]. I was told that either the bootloader or the user should change the
> DT when running in such a non-secure scenario.

ok, this sounds interesting too, as I haven't seen a board that used the dmac 
in question in non-secure mode, yet.

So I guess one way to handle it could be to declare the non-secure dma, but 
set it to a disabled state and use the secure one as default for everything:

		dmac_bus_s: dma-controller@ffb20000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xffb20000 0x4000>;
			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC1>;
			clock-names = "apb_pclk";
		};

		dmac_bus_ns: dma-controller@ffb60000 {
			compatible = "arm,pl330", "arm,primecell";
			reg = <0xffb60000 0x4000>;
			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
			#dma-cells = <1>;
			clocks = <&cru ACLK_DMAC1>;
			clock-names = "apb_pclk";
			status = "disabled";
		};

and a board wanting to use the non-secure variant would then be required to 
override the status and the dma channels in question (this dmac does mainly 
i2s and spdif):

&dmac_bus_s {
	status = "disabled";
}:

&dmac_bus_ns {
	status = "okay";
};

&i2s0 {
	dmas = <&dmac_bus_ns 6>, <&dmac_bus_ns 7>;
};


> [*] https://patchwork.kernel.org/patch/4620251/
> 
> > [0]
> > https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boo
> > t/dts/rk3288.dtsi [1]
> > https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boo
> > t/dts/rk3288-evb.dtsi
> [snip]

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Kever Yang]

On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> Add both the bus and peripheral pl330 dma controllers present in rk3288 socs.
> The first dma controller can change between secure and non-secure mode and is
> left by the bootloader in secure mode, which gets added here.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index 3ef8951..3f39d26 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -62,6 +62,33 @@
>   		};
>   	};
>   
> +	amba {
> +		compatible = "arm,amba-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		dmac1: dma at ffb20000 {
> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0xffb20000 0x4000>;
> +			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			clocks = <&cru ACLK_DMAC1>;
> +			clock-names = "apb_pclk";
> +		};
> +
> +		dmac2: dma at ff250000 {
> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0xff250000 0x4000>;
> +			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			clocks = <&cru ACLK_DMAC2>;
> +			clock-names = "apb_pclk";
> +		};
> +	};
> +
>   	xin24m: oscillator {
>   		compatible = "fixed-clock";
>   		clock-frequency = <24000000>;
Tested-by: Kever Yang <kever.yang@rock-chips.com>

Re: [PATCH 3/4] ARM: dts: rockchip: add rk3288 dma controllers

[Kever Yang]

On 07/30/2014 03:12 AM, Heiko Stuebner wrote:
> Add both the bus and peripheral pl330 dma controllers present in rk3288 socs.
> The first dma controller can change between secure and non-secure mode and is
> left by the bootloader in secure mode, which gets added here.
>
> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
>   arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index 3ef8951..3f39d26 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -62,6 +62,33 @@
>   		};
>   	};
>   
> +	amba {
> +		compatible = "arm,amba-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		dmac1: dma@ffb20000 {
> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0xffb20000 0x4000>;
> +			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			clocks = <&cru ACLK_DMAC1>;
> +			clock-names = "apb_pclk";
> +		};
> +
> +		dmac2: dma@ff250000 {
> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0xff250000 0x4000>;
> +			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			clocks = <&cru ACLK_DMAC2>;
> +			clock-names = "apb_pclk";
> +		};
> +	};
> +
>   	xin24m: oscillator {
>   		compatible = "fixed-clock";
>   		clock-frequency = <24000000>;
Tested-by: Kever Yang <kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] ARM: dts: rockchip: add rk3188 dma controllers

[Heiko Stuebner]

Add both the cpu and peripheral pl330 dma controllers present in rk3188 socs.
The first dma controller can change between secure and non-secure mode and is
left by the bootloader in secure mode, which gets added here.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/boot/dts/rk3xxx.dtsi | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index c6f0561..a6c2860 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -28,6 +28,33 @@
 		i2c4 = &i2c4;
 	};
 
+	amba {
+		compatible = "arm,amba-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		dmac1: dma at 20018000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0x20018000 0x4000>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			clocks = <&cru ACLK_DMA1>;
+			clock-names = "apb_pclk";
+		};
+
+		dmac2: dma at 20078000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0x20078000 0x4000>;
+			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			clocks = <&cru ACLK_DMA2>;
+			clock-names = "apb_pclk";
+		};
+	};
+
 	xin24m: oscillator {
 		compatible = "fixed-clock";
 		clock-frequency = <24000000>;
-- 
2.0.1

[PATCH 4/4] ARM: dts: rockchip: add rk3188 dma controllers

[Heiko Stuebner]

Add both the cpu and peripheral pl330 dma controllers present in rk3188 socs.
The first dma controller can change between secure and non-secure mode and is
left by the bootloader in secure mode, which gets added here.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 arch/arm/boot/dts/rk3xxx.dtsi | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index c6f0561..a6c2860 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -28,6 +28,33 @@
 		i2c4 = &i2c4;
 	};
 
+	amba {
+		compatible = "arm,amba-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		dmac1: dma@20018000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0x20018000 0x4000>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			clocks = <&cru ACLK_DMA1>;
+			clock-names = "apb_pclk";
+		};
+
+		dmac2: dma@20078000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0x20078000 0x4000>;
+			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			clocks = <&cru ACLK_DMA2>;
+			clock-names = "apb_pclk";
+		};
+	};
+
 	xin24m: oscillator {
 		compatible = "fixed-clock";
 		clock-frequency = <24000000>;
-- 
2.0.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] ARM: dts: rockchip: add rk3188 dma controllers

[Sergei Shtylyov]

Hello.

On 07/29/2014 11:12 PM, Heiko Stuebner wrote:

> Add both the cpu and peripheral pl330 dma controllers present in rk3188 socs.
> The first dma controller can change between secure and non-secure mode and is
> left by the bootloader in secure mode, which gets added here.

> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   arch/arm/boot/dts/rk3xxx.dtsi | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)

> diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
> index c6f0561..a6c2860 100644
> --- a/arch/arm/boot/dts/rk3xxx.dtsi
> +++ b/arch/arm/boot/dts/rk3xxx.dtsi
> @@ -28,6 +28,33 @@
>   		i2c4 = &i2c4;
>   	};
>
> +	amba {
> +		compatible = "arm,amba-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		dmac1: dma at 20018000 {

    The ePAPR standard [1] says:

The name of a node should be somewhat generic, reflecting the function of the 
device and not its precise programming model. If appropriate, the name should 
be one of the following choices:

[...]
    ? dma-controller

> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0x20018000 0x4000>;
> +			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			clocks = <&cru ACLK_DMA1>;
> +			clock-names = "apb_pclk";
> +		};
[...]

[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf

WBR, Sergei

Re: [PATCH 4/4] ARM: dts: rockchip: add rk3188 dma controllers

[Sergei Shtylyov]

Hello.

On 07/29/2014 11:12 PM, Heiko Stuebner wrote:

> Add both the cpu and peripheral pl330 dma controllers present in rk3188 socs.
> The first dma controller can change between secure and non-secure mode and is
> left by the bootloader in secure mode, which gets added here.

> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
>   arch/arm/boot/dts/rk3xxx.dtsi | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)

> diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
> index c6f0561..a6c2860 100644
> --- a/arch/arm/boot/dts/rk3xxx.dtsi
> +++ b/arch/arm/boot/dts/rk3xxx.dtsi
> @@ -28,6 +28,33 @@
>   		i2c4 = &i2c4;
>   	};
>
> +	amba {
> +		compatible = "arm,amba-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		dmac1: dma@20018000 {

    The ePAPR standard [1] says:

The name of a node should be somewhat generic, reflecting the function of the 
device and not its precise programming model. If appropriate, the name should 
be one of the following choices:

[...]
    • dma-controller

> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0x20018000 0x4000>;
> +			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			clocks = <&cru ACLK_DMA1>;
> +			clock-names = "apb_pclk";
> +		};
[...]

[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html