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

[Eddie James]

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.

Thanks,
Eddie

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