[PATCH dev-4.19 2/6] clk: aspeed: Setup video engine clocking

[Eddie James]

On 01/08/2019 09:12 AM, Eddie James wrote:
>
>
> On 01/07/2019 08:14 PM, Joel Stanley wrote:
>> On Fri, 4 Jan 2019 at 08:54, Eddie James <eajames at linux.ibm.com> wrote:
>>
>>>   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 */
>>> @@ -317,6 +338,7 @@ struct aspeed_reset {
>>>          [ASPEED_RESET_PECI]     = 10,
>>>          [ASPEED_RESET_I2C]      =  2,
>>>          [ASPEED_RESET_AHB]      =  1,
>>> +       [ASPEED_RESET_VIDEO]    =  6,
>> This is incorrect. You've already added the video reset to the video
>> clock, so the reset will be released by enabling the clock.
>
> I don't quite understand what's wrong about this. Why can't the clock 
> control and reset control both be done independently? Of course the 
> clock control overrides the reset control, but that's fine, presumably?
>
> I'd rather have the reset available separately, so if this must be 
> changed, I'd prefer to drop the reset from the clock description. That 
> way the video driver doesn't need changes.

Nevermind, clock start requires the reset toggle according to the spec, 
so can't drop that...

>
> Thanks,
> Eddie
>
>>
>> If this is the patch that went upstream then upstream needs to be 
>> fixed too.
>>
>