[PATCH dev-5.0 v3 0/7] Enable video engine

[PATCH dev-5.0 v3 0/7] Enable video engine

[Eddie James]

This series adds the necessary clocks for the video engine to the Aspeed clock
driver. It also adds the video engine node to the AST2500 dts and enables that
node in the witherspoon and romulus dts. It also removes the use of the reset
line in the video engine driver.

Changes since v2:
 - make memory-region dts property optional
 - document the memory-region property
 - only reserve 32MB for video memory
 - remove delay after clock enable - my original testing couldn't detect the
   video resolution without that delay, but now I can't repro that problem, so
   I assume it was a test setup issue?

Changes since v1:
 - new reserved memory node for video engine
 - remove reset line from video engine driver
 - no more addition of reset line to clock driver

Eddie James (7):
  media: platform: Aspeed: Remove use of reset line
  media: platform: Aspeed: Make reserved memory optional
  media: dt-bindings: aspeed-video: Add missing memory-region property
  clk: Aspeed: Setup video engine clocking
  ARM: dts: aspeed-g5: Add video engine
  ARM: dts: witherspoon: Enable video engine
  ARM: dts: romulus: Enable video engine

 .../devicetree/bindings/media/aspeed-video.txt     |  6 ++++
 arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts       | 12 +++++++
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts   | 12 +++++++
 arch/arm/boot/dts/aspeed-g5.dtsi                   | 10 ++++++
 drivers/clk/clk-aspeed.c                           | 40 ++++++++++++++++++++--
 drivers/media/platform/aspeed-video.c              | 33 ++++--------------
 6 files changed, 84 insertions(+), 29 deletions(-)

-- 
1.8.3.1

[PATCH dev-5.0 v3 1/7] media: platform: Aspeed: Remove use of reset line

[Eddie James]

The reset line is toggled by enabling the clocks, so it's not necessary
to manually toggle the reset as well.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/media/platform/aspeed-video.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 692e08e..55c55a6 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -14,7 +14,6 @@
 #include <linux/of_irq.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
-#include <linux/reset.h>
 #include <linux/sched.h>
 #include <linux/spinlock.h>
 #include <linux/string.h>
@@ -208,7 +207,6 @@ struct aspeed_video {
 	void __iomem *base;
 	struct clk *eclk;
 	struct clk *vclk;
-	struct reset_control *rst;
 
 	struct device *dev;
 	struct v4l2_ctrl_handler ctrl_handler;
@@ -483,19 +481,10 @@ static void aspeed_video_enable_mode_detect(struct aspeed_video *video)
 	aspeed_video_update(video, VE_SEQ_CTRL, 0, VE_SEQ_CTRL_TRIG_MODE_DET);
 }
 
-static void aspeed_video_reset(struct aspeed_video *video)
-{
-	/* Reset the engine */
-	reset_control_assert(video->rst);
-
-	/* Don't usleep here; function may be called in interrupt context */
-	udelay(100);
-	reset_control_deassert(video->rst);
-}
-
 static void aspeed_video_off(struct aspeed_video *video)
 {
-	aspeed_video_reset(video);
+	/* Disable interrupts */
+	aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
 
 	/* Turn off the relevant clocks */
 	clk_disable_unprepare(video->vclk);
@@ -507,8 +496,6 @@ static void aspeed_video_on(struct aspeed_video *video)
 	/* Turn on the relevant clocks */
 	clk_prepare_enable(video->eclk);
 	clk_prepare_enable(video->vclk);
-
-	aspeed_video_reset(video);
 }
 
 static void aspeed_video_bufs_done(struct aspeed_video *video,
@@ -1464,7 +1451,9 @@ static void aspeed_video_stop_streaming(struct vb2_queue *q)
 		 * Need to force stop any DMA and try and get HW into a good
 		 * state for future calls to start streaming again.
 		 */
-		aspeed_video_reset(video);
+		aspeed_video_off(video);
+		aspeed_video_on(video);
+
 		aspeed_video_init_regs(video);
 
 		aspeed_video_get_resolution(video);
@@ -1619,12 +1608,6 @@ static int aspeed_video_init(struct aspeed_video *video)
 		return PTR_ERR(video->vclk);
 	}
 
-	video->rst = devm_reset_control_get_exclusive(dev, NULL);
-	if (IS_ERR(video->rst)) {
-		dev_err(dev, "Unable to get VE reset\n");
-		return PTR_ERR(video->rst);
-	}
-
 	rc = of_reserved_mem_device_init(dev);
 	if (rc) {
 		dev_err(dev, "Unable to reserve memory\n");
-- 
1.8.3.1

[PATCH dev-5.0 v3 2/7] media: platform: Aspeed: Make reserved memory optional

[Eddie James]

Reserved memory doesn't need to be required; system memory would work
fine.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/media/platform/aspeed-video.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 55c55a6..8144fe3 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -1608,11 +1608,7 @@ static int aspeed_video_init(struct aspeed_video *video)
 		return PTR_ERR(video->vclk);
 	}
 
-	rc = of_reserved_mem_device_init(dev);
-	if (rc) {
-		dev_err(dev, "Unable to reserve memory\n");
-		return rc;
-	}
+	of_reserved_mem_device_init(dev);
 
 	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
 	if (rc) {
-- 
1.8.3.1

[PATCH dev-5.0 v3 3/7] media: dt-bindings: aspeed-video: Add missing memory-region property

[Eddie James]

Missed documenting this property in the initial commit.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 Documentation/devicetree/bindings/media/aspeed-video.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt
index 78b464a..346c2d3 100644
--- a/Documentation/devicetree/bindings/media/aspeed-video.txt
+++ b/Documentation/devicetree/bindings/media/aspeed-video.txt
@@ -14,6 +14,11 @@ Required properties:
 			the VE
  - interrupts:		the interrupt associated with the VE on this platform
 
+Optional properties:
+ - memory-region:
+	phandle to a memory region to allocate from, as defined in
+	Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+
 Example:
 
 video-engine@1e700000 {
@@ -23,4 +28,5 @@ video-engine@1e700000 {
     clock-names = "vclk", "eclk";
     resets = <&syscon ASPEED_RESET_VIDEO>;
     interrupts = <7>;
+    memory-region = <&video_engine_memory>
 };
-- 
1.8.3.1

[PATCH dev-5.0 v3 4/7] clk: Aspeed: Setup video engine clocking

[Eddie James]

Add eclk mux and clock divider table.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/clk/clk-aspeed.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 5961367..133e80c 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -87,7 +87,7 @@ struct aspeed_clk_gate {
 /* TODO: ask Aspeed about the actual parent data */
 static const struct aspeed_gate_data aspeed_gates[] = {
 	/*				 clk rst   name			parent	flags */
-	[ASPEED_CLK_GATE_ECLK] =	{  0, -1, "eclk-gate",		"eclk",	0 }, /* Video Engine */
+	[ASPEED_CLK_GATE_ECLK] =	{  0,  6, "eclk-gate",		"eclk",	0 }, /* Video Engine */
 	[ASPEED_CLK_GATE_GCLK] =	{  1,  7, "gclk-gate",		NULL,	0 }, /* 2D engine */
 	[ASPEED_CLK_GATE_MCLK] =	{  2, -1, "mclk-gate",		"mpll",	CLK_IS_CRITICAL }, /* SDRAM */
 	[ASPEED_CLK_GATE_VCLK] =	{  3,  6, "vclk-gate",		NULL,	0 }, /* Video Capture */
@@ -113,6 +113,24 @@ struct aspeed_clk_gate {
 	[ASPEED_CLK_GATE_LHCCLK] =	{ 28, -1, "lhclk-gate",		"lhclk", 0 }, /* LPC master/LPC+ */
 };
 
+static const char * const eclk_parent_names[] = {
+	"mpll",
+	"hpll",
+	"dpll",
+};
+
+static const struct clk_div_table ast2500_eclk_div_table[] = {
+	{ 0x0, 2 },
+	{ 0x1, 2 },
+	{ 0x2, 3 },
+	{ 0x3, 4 },
+	{ 0x4, 5 },
+	{ 0x5, 6 },
+	{ 0x6, 7 },
+	{ 0x7, 8 },
+	{ 0 }
+};
+
 static const struct clk_div_table ast2500_mac_div_table[] = {
 	{ 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */
 	{ 0x1, 4 },
@@ -192,18 +210,21 @@ static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val)
 
 struct aspeed_clk_soc_data {
 	const struct clk_div_table *div_table;
+	const struct clk_div_table *eclk_div_table;
 	const struct clk_div_table *mac_div_table;
 	struct clk_hw *(*calc_pll)(const char *name, u32 val);
 };
 
 static const struct aspeed_clk_soc_data ast2500_data = {
 	.div_table = ast2500_div_table,
+	.eclk_div_table = ast2500_eclk_div_table,
 	.mac_div_table = ast2500_mac_div_table,
 	.calc_pll = aspeed_ast2500_calc_pll,
 };
 
 static const struct aspeed_clk_soc_data ast2400_data = {
 	.div_table = ast2400_div_table,
+	.eclk_div_table = ast2400_div_table,
 	.mac_div_table = ast2400_div_table,
 	.calc_pll = aspeed_ast2400_calc_pll,
 };
@@ -522,6 +543,22 @@ static int aspeed_clk_probe(struct platform_device *pdev)
 		return PTR_ERR(hw);
 	aspeed_clk_data->hws[ASPEED_CLK_24M] = hw;
 
+	hw = clk_hw_register_mux(dev, "eclk-mux", eclk_parent_names,
+				 ARRAY_SIZE(eclk_parent_names), 0,
+				 scu_base + ASPEED_CLK_SELECTION, 2, 0x3, 0,
+				 &aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_clk_data->hws[ASPEED_CLK_ECLK_MUX] = hw;
+
+	hw = clk_hw_register_divider_table(dev, "eclk", "eclk-mux", 0,
+					   scu_base + ASPEED_CLK_SELECTION, 28,
+					   3, 0, soc_data->eclk_div_table,
+					   &aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw;
+
 	/*
 	 * TODO: There are a number of clocks that not included in this driver
 	 * as more information is required:
@@ -531,7 +568,6 @@ static int aspeed_clk_probe(struct platform_device *pdev)
 	 *   RGMII
 	 *   RMII
 	 *   UART[1..5] clock source mux
-	 *   Video Engine (ECLK) mux and clock divider
 	 */
 
 	for (i = 0; i < ARRAY_SIZE(aspeed_gates); i++) {
-- 
1.8.3.1

[PATCH dev-5.0 v3 5/7] ARM: dts: aspeed-g5: Add video engine

[Eddie James]

Add a node to describe the video engine on the AST2500.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/boot/dts/aspeed-g5.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index a79e01f..d11c078 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -252,6 +252,16 @@
 				status = "disabled";
 			};
 
+			video: video@1e700000 {
+				compatible = "aspeed,ast2500-video-engine";
+				reg = <0x1e700000 0x1000>;
+				clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
+					 <&syscon ASPEED_CLK_GATE_ECLK>;
+				clock-names = "vclk", "eclk";
+				interrupts = <7>;
+				status = "disabled";
+			};
+
 			sram: sram@1e720000 {
 				compatible = "mmio-sram";
 				reg = <0x1e720000 0x9000>;	// 36K
-- 
1.8.3.1

[PATCH dev-5.0 v3 6/7] ARM: dts: witherspoon: Enable video engine

[Eddie James]

Enable the video engine and add it's optional reserved memory region.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index 1cdc96d..e09863c 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -33,6 +33,13 @@
 			compatible = "shared-dma-pool";
 			reusable;
 		};
+
+		video_engine_memory: jpegbuffer {
+			size = <0x02000000>;	/* 32MM */
+			alignment = <0x01000000>;
+			compatible = "shared-dma-pool";
+			reusable;
+		};
 	};
 
 	gpio-keys {
@@ -664,4 +671,9 @@
 	status = "okay";
 };
 
+&video {
+	status = "okay";
+	memory-region = <&video_engine_memory>;
+};
+
 #include "ibm-power9-dual.dtsi"
-- 
1.8.3.1

[PATCH dev-5.0 v3 7/7] ARM: dts: romulus: Enable video engine

[Eddie James]

Enable the video engine and add it's optional reserved memory region.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
index ee1a460..8c47c60 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
@@ -42,6 +42,13 @@
 			compatible = "shared-dma-pool";
 			reusable;
 		};
+
+		video_engine_memory: jpegbuffer {
+			size = <0x02000000>;	/* 32M */
+			alignment = <0x01000000>;
+			compatible = "shared-dma-pool";
+			reusable;
+		};
 	};
 
 	leds {
@@ -315,4 +322,9 @@
      memory-region = <&gfx_memory>;
 };
 
+&video {
+	status = "okay";
+	memory-region = <&video_engine_memory>;
+};
+
 #include "ibm-power9-dual.dtsi"
-- 
1.8.3.1

Re: [PATCH dev-5.0 v3 4/7] clk: Aspeed: Setup video engine clocking

[Jae Hyun Yoo]

On 4/1/2019 1:23 PM, Eddie James wrote:
> Add eclk mux and clock divider table.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>   drivers/clk/clk-aspeed.c | 40 ++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 5961367..133e80c 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -87,7 +87,7 @@ struct aspeed_clk_gate {
>   /* TODO: ask Aspeed about the actual parent data */
>   static const struct aspeed_gate_data aspeed_gates[] = {
>   	/*				 clk rst   name			parent	flags */
> -	[ASPEED_CLK_GATE_ECLK] =	{  0, -1, "eclk-gate",		"eclk",	0 }, /* Video Engine */
> +	[ASPEED_CLK_GATE_ECLK] =	{  0,  6, "eclk-gate",		"eclk",	0 }, /* Video Engine */
>   	[ASPEED_CLK_GATE_GCLK] =	{  1,  7, "gclk-gate",		NULL,	0 }, /* 2D engine */
>   	[ASPEED_CLK_GATE_MCLK] =	{  2, -1, "mclk-gate",		"mpll",	CLK_IS_CRITICAL }, /* SDRAM */
>   	[ASPEED_CLK_GATE_VCLK] =	{  3,  6, "vclk-gate",		NULL,	0 }, /* Video Capture */

Hi Eddie,

I already left a comment on it but you probably didn't see that.
With this change, both eclk and vclk will be coupled with reset bit '6'.
Actually, the reset bit 6 is for video engine so this change seems
correct, but you should replace reset bit of vclk with -1 instead
otherwise the video engine reset will be triggered twice and it will
take at least total 20ms of delay time which is a bad idea in case if
aspeed_clk_enable is called from interrupt context.

Test it again after changing the vclk rst setting to '-1' on top of this
patch series. It worked well in my H/W.

Cheers,
Jae

> @@ -113,6 +113,24 @@ struct aspeed_clk_gate {
>   	[ASPEED_CLK_GATE_LHCCLK] =	{ 28, -1, "lhclk-gate",		"lhclk", 0 }, /* LPC master/LPC+ */
>   };
>   
> +static const char * const eclk_parent_names[] = {
> +	"mpll",
> +	"hpll",
> +	"dpll",
> +};
> +
> +static const struct clk_div_table ast2500_eclk_div_table[] = {
> +	{ 0x0, 2 },
> +	{ 0x1, 2 },
> +	{ 0x2, 3 },
> +	{ 0x3, 4 },
> +	{ 0x4, 5 },
> +	{ 0x5, 6 },
> +	{ 0x6, 7 },
> +	{ 0x7, 8 },
> +	{ 0 }
> +};
> +
>   static const struct clk_div_table ast2500_mac_div_table[] = {
>   	{ 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */
>   	{ 0x1, 4 },
> @@ -192,18 +210,21 @@ static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val)
>   
>   struct aspeed_clk_soc_data {
>   	const struct clk_div_table *div_table;
> +	const struct clk_div_table *eclk_div_table;
>   	const struct clk_div_table *mac_div_table;
>   	struct clk_hw *(*calc_pll)(const char *name, u32 val);
>   };
>   
>   static const struct aspeed_clk_soc_data ast2500_data = {
>   	.div_table = ast2500_div_table,
> +	.eclk_div_table = ast2500_eclk_div_table,
>   	.mac_div_table = ast2500_mac_div_table,
>   	.calc_pll = aspeed_ast2500_calc_pll,
>   };
>   
>   static const struct aspeed_clk_soc_data ast2400_data = {
>   	.div_table = ast2400_div_table,
> +	.eclk_div_table = ast2400_div_table,
>   	.mac_div_table = ast2400_div_table,
>   	.calc_pll = aspeed_ast2400_calc_pll,
>   };
> @@ -522,6 +543,22 @@ static int aspeed_clk_probe(struct platform_device *pdev)
>   		return PTR_ERR(hw);
>   	aspeed_clk_data->hws[ASPEED_CLK_24M] = hw;
>   
> +	hw = clk_hw_register_mux(dev, "eclk-mux", eclk_parent_names,
> +				 ARRAY_SIZE(eclk_parent_names), 0,
> +				 scu_base + ASPEED_CLK_SELECTION, 2, 0x3, 0,
> +				 &aspeed_clk_lock);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +	aspeed_clk_data->hws[ASPEED_CLK_ECLK_MUX] = hw;
> +
> +	hw = clk_hw_register_divider_table(dev, "eclk", "eclk-mux", 0,
> +					   scu_base + ASPEED_CLK_SELECTION, 28,
> +					   3, 0, soc_data->eclk_div_table,
> +					   &aspeed_clk_lock);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +	aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw;
> +
>   	/*
>   	 * TODO: There are a number of clocks that not included in this driver
>   	 * as more information is required:
> @@ -531,7 +568,6 @@ static int aspeed_clk_probe(struct platform_device *pdev)
>   	 *   RGMII
>   	 *   RMII
>   	 *   UART[1..5] clock source mux
> -	 *   Video Engine (ECLK) mux and clock divider
>   	 */
>   
>   	for (i = 0; i < ARRAY_SIZE(aspeed_gates); i++) {
> 

Re: [PATCH dev-5.0 v3 4/7] clk: Aspeed: Setup video engine clocking

[Eddie James]

On 4/1/19 3:48 PM, Jae Hyun Yoo wrote:
> On 4/1/2019 1:23 PM, Eddie James wrote:
>> Add eclk mux and clock divider table.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   drivers/clk/clk-aspeed.c | 40 ++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
>> index 5961367..133e80c 100644
>> --- a/drivers/clk/clk-aspeed.c
>> +++ b/drivers/clk/clk-aspeed.c
>> @@ -87,7 +87,7 @@ struct aspeed_clk_gate {
>>   /* TODO: ask Aspeed about the actual parent data */
>>   static const struct aspeed_gate_data aspeed_gates[] = {
>>       /*                 clk rst   name            parent flags */
>> -    [ASPEED_CLK_GATE_ECLK] =    {  0, -1, "eclk-gate", "eclk",    0 
>> }, /* Video Engine */
>> +    [ASPEED_CLK_GATE_ECLK] =    {  0,  6, "eclk-gate", "eclk",    0 
>> }, /* Video Engine */
>>       [ASPEED_CLK_GATE_GCLK] =    {  1,  7, "gclk-gate", NULL,    0 
>> }, /* 2D engine */
>>       [ASPEED_CLK_GATE_MCLK] =    {  2, -1, "mclk-gate", "mpll",    
>> CLK_IS_CRITICAL }, /* SDRAM */
>>       [ASPEED_CLK_GATE_VCLK] =    {  3,  6, "vclk-gate", NULL,    0 
>> }, /* Video Capture */
>
> Hi Eddie,
>
> I already left a comment on it but you probably didn't see that.
> With this change, both eclk and vclk will be coupled with reset bit '6'.
> Actually, the reset bit 6 is for video engine so this change seems
> correct, but you should replace reset bit of vclk with -1 instead
> otherwise the video engine reset will be triggered twice and it will
> take at least total 20ms of delay time which is a bad idea in case if
> aspeed_clk_enable is called from interrupt context.
>
> Test it again after changing the vclk rst setting to '-1' on top of this
> patch series. It worked well in my H/W.


Sorry, I had seen it and then forgotten about it... thanks Jae, will 
include.

Eddie


>
> Cheers,
> Jae
>
>> @@ -113,6 +113,24 @@ struct aspeed_clk_gate {
>>       [ASPEED_CLK_GATE_LHCCLK] =    { 28, -1, "lhclk-gate",        
>> "lhclk", 0 }, /* LPC master/LPC+ */
>>   };
>>   +static const char * const eclk_parent_names[] = {
>> +    "mpll",
>> +    "hpll",
>> +    "dpll",
>> +};
>> +
>> +static const struct clk_div_table ast2500_eclk_div_table[] = {
>> +    { 0x0, 2 },
>> +    { 0x1, 2 },
>> +    { 0x2, 3 },
>> +    { 0x3, 4 },
>> +    { 0x4, 5 },
>> +    { 0x5, 6 },
>> +    { 0x6, 7 },
>> +    { 0x7, 8 },
>> +    { 0 }
>> +};
>> +
>>   static const struct clk_div_table ast2500_mac_div_table[] = {
>>       { 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */
>>       { 0x1, 4 },
>> @@ -192,18 +210,21 @@ static struct clk_hw 
>> *aspeed_ast2500_calc_pll(const char *name, u32 val)
>>     struct aspeed_clk_soc_data {
>>       const struct clk_div_table *div_table;
>> +    const struct clk_div_table *eclk_div_table;
>>       const struct clk_div_table *mac_div_table;
>>       struct clk_hw *(*calc_pll)(const char *name, u32 val);
>>   };
>>     static const struct aspeed_clk_soc_data ast2500_data = {
>>       .div_table = ast2500_div_table,
>> +    .eclk_div_table = ast2500_eclk_div_table,
>>       .mac_div_table = ast2500_mac_div_table,
>>       .calc_pll = aspeed_ast2500_calc_pll,
>>   };
>>     static const struct aspeed_clk_soc_data ast2400_data = {
>>       .div_table = ast2400_div_table,
>> +    .eclk_div_table = ast2400_div_table,
>>       .mac_div_table = ast2400_div_table,
>>       .calc_pll = aspeed_ast2400_calc_pll,
>>   };
>> @@ -522,6 +543,22 @@ static int aspeed_clk_probe(struct 
>> platform_device *pdev)
>>           return PTR_ERR(hw);
>>       aspeed_clk_data->hws[ASPEED_CLK_24M] = hw;
>>   +    hw = clk_hw_register_mux(dev, "eclk-mux", eclk_parent_names,
>> +                 ARRAY_SIZE(eclk_parent_names), 0,
>> +                 scu_base + ASPEED_CLK_SELECTION, 2, 0x3, 0,
>> +                 &aspeed_clk_lock);
>> +    if (IS_ERR(hw))
>> +        return PTR_ERR(hw);
>> +    aspeed_clk_data->hws[ASPEED_CLK_ECLK_MUX] = hw;
>> +
>> +    hw = clk_hw_register_divider_table(dev, "eclk", "eclk-mux", 0,
>> +                       scu_base + ASPEED_CLK_SELECTION, 28,
>> +                       3, 0, soc_data->eclk_div_table,
>> +                       &aspeed_clk_lock);
>> +    if (IS_ERR(hw))
>> +        return PTR_ERR(hw);
>> +    aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw;
>> +
>>       /*
>>        * TODO: There are a number of clocks that not included in this 
>> driver
>>        * as more information is required:
>> @@ -531,7 +568,6 @@ static int aspeed_clk_probe(struct 
>> platform_device *pdev)
>>        *   RGMII
>>        *   RMII
>>        *   UART[1..5] clock source mux
>> -     *   Video Engine (ECLK) mux and clock divider
>>        */
>>         for (i = 0; i < ARRAY_SIZE(aspeed_gates); i++) {
>>
>