[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 at 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++) {
>>
>