[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?

Hey Joel,

After some more testing, the driver NEEDS to have the reset available 
separately. The problem is that the clock driver doesn't enable reset 
before the clocks turn off, resulting in odd behavior. For example, I 
get endless IRQs and can't disable them by writing the engine registers, 
since the clocks are off.

What do you suggest here?

Thanks,
Eddie

>
> 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.
>
> Thanks,
> Eddie
>
>>
>> If this is the patch that went upstream then upstream needs to be 
>> fixed too.
>>
>