ARM: rockchip: add dma support
Subsystems (5)
Heiko Stuebner
maintainercover ARM: rockchip: add dma support
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(+)
Signature · 1 line
2.0.1
Heiko Stuebner
maintainerPATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
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.
--- 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(+)
drivers/clk/rockchip/clk-rk3188.c +7 -0
@@ -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 );
drivers/clk/rockchip/clk-rk3288.c +7 -0
@@ -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 );
drivers/clk/rockchip/clk.c +13 -0
@@ -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); + } +}
drivers/clk/rockchip/clk.h +1 -0
@@ -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)
Signature · 1 line
2.0.1
PATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
Quoting Heiko Stuebner (2014-07-29 12:12:05)
Quoted 19 lines
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 <[email protected]>--- 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(+)drivers/clk/rockchip/clk-rk3188.c +0
-0
drivers/clk/rockchip/clk-rk3188.c static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = { · line 604
Quoted 6 lines
drivers/clk/rockchip/clk-rk3188.c +3
-0@@ -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
Quoted 75 lines
drivers/clk/rockchip/clk-rk3188.c +4
-0+}; +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 );drivers/clk/rockchip/clk-rk3288.c +7
-0@@ -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 );drivers/clk/rockchip/clk.c +13
-0@@ -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); + } +}drivers/clk/rockchip/clk.h +1
-0@@ -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)Signature · 1 line
2.0.1
Heiko Stübner
maintainerPATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
Hi Mike,
Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
Quoted 28 lines
Quoting Heiko Stuebner (2014-07-29 12:12:05)
Quoted 26 lines
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 <[email protected]>---
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(+)(quoted diff) +0
-0
b/drivers/clk/rockchip/clk-rk3188.cindex a83a6d8..5aef277 100644drivers/clk/rockchip/clk-rk3188.c +0
-0@@ -599,6 +599,11 @@ static struct rockchip_clk_branchrk3188_clk_branches[]
__initdata= {>GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),>};
Quoted 7 lines
drivers/clk/rockchip/clk-rk3188.c static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = { · line 604
Quoted 3 lines
drivers/clk/rockchip/clk-rk3188.c +3
-0+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_getand enabling them the normal way viaclk_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
PATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
Quoting Heiko St?bner (2014-07-31 16:29:34)
Quoted 39 lines
Hi Mike,
Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
Quoted 28 lines
Quoting Heiko Stuebner (2014-07-29 12:12:05)
Quoted 26 lines
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 <[email protected]>---
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(+)(quoted diff) +0
-0
b/drivers/clk/rockchip/clk-rk3188.cindex a83a6d8..5aef277 100644drivers/clk/rockchip/clk-rk3188.c +0
-0@@ -599,6 +599,11 @@ static struct rockchip_clk_branchrk3188_clk_branches[]
__initdata= {>GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),>};
Quoted 7 lines
drivers/clk/rockchip/clk-rk3188.c static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = { · line 604
Quoted 3 lines
drivers/clk/rockchip/clk-rk3188.c +3
-0+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_getand enabling them the normal way viaclk_enable?
Quoted 6 lines
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
Quoted 3 lines
Heiko
Heiko Stübner
maintainerPATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
Quoted 53 lines
Quoting Heiko St?bner (2014-07-31 16:29:34)
Quoted 44 lines
Hi Mike,
Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
Quoted 33 lines
Quoting Heiko Stuebner (2014-07-29 12:12:05)
Quoted 31 lines
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 <[email protected]>---
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(+)(quoted diff) +0
-0
b/drivers/clk/rockchip/clk-rk3188.cindex a83a6d8..5aef277 100644drivers/clk/rockchip/clk-rk3188.c +0
-0@@ -599,6 +599,11 @@ static struct rockchip_clk_branchrk3188_clk_branches[]
__initdata= {>GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),>};
Quoted 7 lines
drivers/clk/rockchip/clk-rk3188.c static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = { · line 604
Quoted 3 lines
drivers/clk/rockchip/clk-rk3188.c +3
-0+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_getand enabling them the normal way viaclk_enable?Quoted 6 lines
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).
Quoted 2 lines
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
PATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
Heiko,
Quoted 62 lines · Heiko St?bner
Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
Quoted 53 lines
Quoting Heiko St?bner (2014-07-31 16:29:34)
Quoted 44 lines
Hi Mike,
Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
Quoted 33 lines
Quoting Heiko Stuebner (2014-07-29 12:12:05)
Quoted 31 lines
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 <[email protected]>---
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(+)(quoted diff) +0
-0
b/drivers/clk/rockchip/clk-rk3188.cindex a83a6d8..5aef277 100644drivers/clk/rockchip/clk-rk3188.c +0
-0@@ -599,6 +599,11 @@ static struct rockchip_clk_branchrk3188_clk_branches[]
__initdata= {>GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),>};
Quoted 7 lines
drivers/clk/rockchip/clk-rk3188.c static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = { · line 604
Quoted 3 lines
drivers/clk/rockchip/clk-rk3188.c +3
-0+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_getand enabling them the normal way viaclk_enable?Quoted 6 lines
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).
Quoted 2 lines
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)
Quoted 4 lines
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:
Heiko Stübner
maintainerPATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
Quoted 71 lines
Heiko,
Quoted 68 lines · Heiko St?bner
Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
Quoted 59 lines
Quoting Heiko St?bner (2014-07-31 16:29:34)
Quoted 48 lines
Hi Mike,
Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
Quoted 35 lines
Quoting Heiko Stuebner (2014-07-29 12:12:05)
Quoted 33 lines
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 <[email protected]>---
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(+)(quoted diff) +0
-0
b/drivers/clk/rockchip/clk-rk3188.cindex a83a6d8..5aef277 100644drivers/clk/rockchip/clk-rk3188.c +0
-0@@ -599,6 +599,11 @@ static struct rockchip_clk_branchrk3188_clk_branches[]
__initdata= {>GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),>};
Quoted 9 lines
drivers/clk/rockchip/clk-rk3188.c static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = { · line 604
Quoted 3 lines
drivers/clk/rockchip/clk-rk3188.c +3
-0+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 viaclk_getand enabling them the normal way viaclk_enable?Quoted 8 lines
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).Quoted 2 lines
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)
Quoted 8 lines
Quoted 4 lines
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.
Quoted 4 lines
In any case, this patch fixes a hang at boot when using the PWM driver that just landed, so:
Tested-by Doug Anderson <[email protected]>
thanks
Heiko
PATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
Quoted 75 lines · Heiko St?bner
Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
Quoted 67 lines
Heiko,
Quoted 64 lines · Heiko St?bner
Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
Quoted 56 lines
Quoting Heiko St?bner (2014-07-31 16:29:34)
Quoted 46 lines
Hi Mike,
Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
Quoted 35 lines
Quoting Heiko Stuebner (2014-07-29 12:12:05)
Quoted 33 lines
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 <[email protected]>---
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(+)(quoted diff) +0
-0
b/drivers/clk/rockchip/clk-rk3188.cindex a83a6d8..5aef277 100644drivers/clk/rockchip/clk-rk3188.c +0
-0@@ -599,6 +599,11 @@ static struct rockchip_clk_branchrk3188_clk_branches[]
__initdata= {>GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),>};Quoted 8 lines
drivers/clk/rockchip/clk-rk3188.c
Quoted 3 lines
drivers/clk/rockchip/clk-rk3188.c +3
-0+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 viaclk_getand enabling them the normal way viaclk_enable?Quoted 8 lines
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).Quoted 2 lines
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)
Quoted 7 lines
Quoted 4 lines
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?
Quoted 8 lines
hmm looking at the core clock tree, I wouldn't think so.
The only intersection between the ddr-clk,
aclk_cpuandaclk_periis 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 theaclk_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
Quoted 13 lines
Quoted 6 lines
In any case, this patch fixes a hang at boot when using the PWM driver that just landed, so:
Tested-by Doug Anderson <[email protected]>thanks
Heiko
Heiko Stübner
maintainerPATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
Hi Kever,
Am Montag, 11. August 2014, 18:03:33 schrieb Kever Yang:
Quoted 113 lines · Heiko St?bner
Quoted 87 lines
Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
Quoted 80 lines
Heiko,
Quoted 70 lines · Heiko St?bner
Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
Quoted 59 lines
Quoting Heiko St?bner (2014-07-31 16:29:34)
Quoted 48 lines
Hi Mike,
Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
Quoted 35 lines
Quoting Heiko Stuebner (2014-07-29 12:12:05)
Quoted 33 lines
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 <[email protected]>---
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(+)(quoted diff) +0
-0
b/drivers/clk/rockchip/clk-rk3188.cindex a83a6d8..5aef277 100644drivers/clk/rockchip/clk-rk3188.c +0
-0@@ -599,6 +599,11 @@ static struct rockchip_clk_branchrk3188_clk_branches[]
__initdata= {>GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0, RK2928_CLKGATE_CON(8), 13, GFLAGS),>};Quoted 9 lines
drivers/clk/rockchip/clk-rk3188.c
Quoted 3 lines
drivers/clk/rockchip/clk-rk3188.c +3
-0+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 viaclk_getand enabling them the normal way viaclk_enable?Quoted 8 lines
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).Quoted 2 lines
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)Quoted 6 lines
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.Quoted 3 lines
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.
Quoted 7 lines
The only intersection between the ddr-clk,
aclk_cpuandaclk_periis 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 inpd_busactually, some clocks not listed
as a module clock will be needed, like cpu I/D bus fetch instruction/data from dram via
bus based onaclk_cpu. For this situation, can we use a dummy clock to hold theaclk_cpunot 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.
Quoted 3 lines
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
Quoted 12 lines
-Kever
Quoted 9 lines
Quoted 4 lines
In any case, this patch fixes a hang at boot when using the PWM driver that just landed, so:
Tested-by Doug Anderson <[email protected]>thanks
Heiko
PATCH 1/4 clk: rockchip: protect critical clocks from getting disabled
In reply to Heiko StuebnerHeiko,
Quoted 90 lines · 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 <[email protected]>--- 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(+)drivers/clk/rockchip/clk-rk3188.c +7
-0@@ -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 );drivers/clk/rockchip/clk-rk3288.c +7
-0@@ -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 );drivers/clk/rockchip/clk.c +13
-0@@ -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); + } +}drivers/clk/rockchip/clk.h +0
-0
drivers/clk/rockchip/clk.h void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list, · line 335
Quoted 7 lines
drivers/clk/rockchip/clk.h +1
-0@@ -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)
Heiko Stuebner
maintainerPATCH 2/4 ARM: rockchip: enable the AMBA bus
In reply to Heiko StuebnerThis is needed to access the pl330 dma controllers on Rockchip SoCs.
--- arch/arm/mach-rockchip/Kconfig | 1 + 1 file changed, 1 insertion(+)
arch/arm/mach-rockchip/Kconfig +1 -0
@@ -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
Signature · 1 line
2.0.1
PATCH 2/4 ARM: rockchip: enable the AMBA bus
Heiko:
Quoted 12 lines · Heiko Stuebner
This is needed to access the pl330 dma controllers on Rockchip SoCs.
Signed-off-by Heiko Stuebner <[email protected]>--- arch/arm/mach-rockchip/Kconfig | 1 + 1 file changed, 1 insertion(+)arch/arm/mach-rockchip/Kconfig +0
-0
arch/arm/mach-rockchip/Kconfig config ARCH_ROCKCHIP · line 7
Quoted 4 lines
arch/arm/mach-rockchip/Kconfig +1
-0@@ -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_DMAQuoted 3 lines
arch/arm/mach-rockchip/Kconfig +0
-0select ARM_GIC select CACHE_L2X0 select HAVE_ARM_ARCH_TIMER
Heiko Stübner
maintainerPATCH 2/4 ARM: rockchip: enable the AMBA bus
Am Montag, 11. August 2014, 11:35:15 schrieb Kever Yang:
Quoted 21 lines
Heiko:
Quoted 18 lines · Heiko Stuebner
This is needed to access the pl330 dma controllers on Rockchip SoCs.
Signed-off-by Heiko Stuebner <[email protected]>---
arch/arm/mach-rockchip/Kconfig | 1 + 1 file changed, 1 insertion(+)(quoted diff) +0
-0b/arch/arm/mach-rockchip/Kconfig index d168669..ac5803c 100644
arch/arm/mach-rockchip/Kconfig +0
-0@@ -4,6 +4,7 @@ config ARCH_ROCKCHIP select PINCTRL_ROCKCHIP select ARCH_HAS_RESET_CONTROLLER select ARCH_REQUIRE_GPIOLIB
Quoted 5 lines
arch/arm/mach-rockchip/Kconfig config ARCH_ROCKCHIP · line 7
Quoted 1 line
arch/arm/mach-rockchip/Kconfig +1
-0+ select ARM_AMBAThese 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
PATCH 2/4 ARM: rockchip: enable the AMBA bus
In reply to Heiko StuebnerHeiko,
Quoted 6 lines · Heiko Stuebner
This is needed to access the pl330 dma controllers on Rockchip SoCs.
Signed-off-by Heiko Stuebner <[email protected]>--- arch/arm/mach-rockchip/Kconfig | 1 + 1 file changed, 1 insertion(+)
PATCH 2/4 ARM: rockchip: enable the AMBA bus
In reply to Heiko StuebnerQuoted 12 lines · Heiko Stuebner
This is needed to access the pl330 dma controllers on Rockchip SoCs.
Signed-off-by Heiko Stuebner <[email protected]>--- arch/arm/mach-rockchip/Kconfig | 1 + 1 file changed, 1 insertion(+)arch/arm/mach-rockchip/Kconfig +0
-0
arch/arm/mach-rockchip/Kconfig config ARCH_ROCKCHIP · line 10
Quoted 7 lines
arch/arm/mach-rockchip/Kconfig +1
-0@@ -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
Heiko Stuebner
maintainerPATCH 3/4 ARM: dts: rockchip: add rk3288 dma controllers
In reply to Heiko StuebnerAdd 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.
--- arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
arch/arm/boot/dts/rk3288.dtsi +27 -0
@@ -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>;
Signature · 1 line
2.0.1
PATCH 3/4 ARM: dts: rockchip: add rk3288 dma controllers
Heiko,
Quoted 8 lines · 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 <[email protected]>--- 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.
Quoted 5 lines
arch/arm/boot/dts/rk3288.dtsi +0
-0
arch/arm/boot/dts/rk3288.dtsi line 71
Quoted 10 lines
arch/arm/boot/dts/rk3288.dtsi +7
-0@@ -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".
arch/arm/boot/dts/rk3288.dtsi line 81
Quoted 10 lines
arch/arm/boot/dts/rk3288.dtsi +10
-0+ 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?
Quoted 16 lines
arch/arm/boot/dts/rk3288.dtsi +10
-1+ 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
Heiko Stübner
maintainerPATCH 3/4 ARM: dts: rockchip: add rk3288 dma controllers
Am Montag, 11. August 2014, 10:01:08 schrieb Doug Anderson:
Quoted 26 lines
Heiko,
Quoted 9 lines · 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 <[email protected]>---
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.
Quoted 10 lines
arch/arm/boot/dts/rk3288.dtsi +0
-0@@ -62,6 +62,33 @@ }; };
Quoted 14 lines
arch/arm/boot/dts/rk3288.dtsi line 71
Quoted 7 lines
arch/arm/boot/dts/rk3288.dtsi +7
-0+ 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 atff250000"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
Quoted 12 lines
arch/arm/boot/dts/rk3288.dtsi line 81
Quoted 11 lines
arch/arm/boot/dts/rk3288.dtsi +10
-0+ 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 {
Quoted 2 lines
nit: when we have easy addresses to compare to, we should probably sort by address?
correct :-)
Quoted 19 lines
Quoted 19 lines
arch/arm/boot/dts/rk3288.dtsi +10
-0+ 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
Am 11.08.2014 19:01, schrieb Heiko St?bner:
Quoted 55 lines
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.dtsilooks 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"; }; };
Quoted 3 lines
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/
Quoted 2 lines
[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]
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg
Heiko Stübner
maintainerPATCH 3/4 ARM: dts: rockchip: add rk3288 dma controllers
Am Montag, 11. August 2014, 19:37:29 schrieb Andreas F?rber:
Quoted 74 lines
Am 11.08.2014 19:01, schrieb Heiko St?bner:
Quoted 73 lines
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.dtsilooks 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"; }; };
Quoted 7 lines
Quoted 3 lines
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_sanddmac1_nslike 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 :-) .
Quoted 3 lines
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>;
};Quoted 8 lines
[*] https://patchwork.kernel.org/patch/4620251/
Quoted 5 lines
[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]
PATCH 3/4 ARM: dts: rockchip: add rk3288 dma controllers
In reply to Heiko StuebnerQuoted 37 lines · 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 <[email protected]>--- arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)arch/arm/boot/dts/rk3288.dtsi +20
-0@@ -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>,
arch/arm/boot/dts/rk3288.dtsi line 94
Quoted 10 lines
arch/arm/boot/dts/rk3288.dtsi +7
-0+ <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>;
Heiko Stuebner
maintainerPATCH 4/4 ARM: dts: rockchip: add rk3188 dma controllers
In reply to Heiko StuebnerAdd 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.
--- arch/arm/boot/dts/rk3xxx.dtsi | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
arch/arm/boot/dts/rk3xxx.dtsi +27 -0
@@ -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>;
Signature · 1 line
2.0.1
PATCH 4/4 ARM: dts: rockchip: add rk3188 dma controllers
Hello.
Quoted 14 lines · 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 <[email protected]>--- arch/arm/boot/dts/rk3xxx.dtsi | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)arch/arm/boot/dts/rk3xxx.dtsi +0
-0
arch/arm/boot/dts/rk3xxx.dtsi line 37
Quoted 10 lines
arch/arm/boot/dts/rk3xxx.dtsi +7
-0@@ -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
arch/arm/boot/dts/rk3xxx.dtsi line 45
Quoted 8 lines
arch/arm/boot/dts/rk3xxx.dtsi +8
-0+ 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