Re: [RFC PATCH v3 2/2] clk: exynos5420: Make sure MDMA0 clock is enabled during suspend

[Javier Martinez Canillas]

Hello Tomasz,

On 04/07/2015 02:46 PM, Tomasz Figa wrote:
> 2015-04-07 13:56 GMT+02:00 Javier Martinez Canillas
> <javier.martinez@xxxxxxxxxxxxxxx>:
>> So I disabled the sss clock before trying a S2R:
>>
>> # devmem 0x10018800 32 0xFFFFFFFB
>> (CLK_SSS in CLK_GATE_IP_G2D is gated)
>>
>> and S2R worked anyways but I see that CLK_GATE_IP_G2D is reset to
>> its default value on S2R so maybe that is why it works anyways?
> 
> Does the driver restore its value on resume (i.e. has it in the
> save/restore array)? Remember that suspend causes all clock registers
> to be reset. Then some of them will be configured by the lowest

No, GATE_IP_G2D is not in the exynos5x_gate_clks array so it looses
the kernel after a suspend/resume cycle.

> bootloader stage after wake-up reset, but the kernel needs to restore
> all of them.
> 

I see, thanks for the clarification. Then I think that is a bug and
GATE_IP_G2D needs to be added to the list of clocks to be saved and
restored right? That's a separate issue from our current S2R problem
though so it can be done later as a separate patch.

>>
>> # devmem 0x10018800
>> 0xFFFFFFFF (all CLK_GATE_IP_G2D clocks enabled including CLK_SSS)
>>
>> Does this shed any more light? Could the problem be that the sss
>> clock parent (aclk266_g2d) is gated during S2R? Is the SSS module
>> required for S2R or is just that CLK_SSS prevents the parent to
>> be gated and so it is another red herring?
> 
> Does the board use secure firmware? If yes, it might require to do
> some encryption on suspend, so if the firmware is broken and doesn't
> control the clock itself, it might need the SSS clock to be running,
> when the SLEEP SMC operation is called.
> 

No, the Chromebooks don't use secure firmware AFAIK.

> Anyway, I just realized that Exynos4 also need several clocks to be
> ungated on suspend and this is handled by code [1] based on arrays
> [2].
> 
> [1] http://lxr.free-electrons.com/source/drivers/clk/samsung/clk-exynos4.c#L309
> [2] http://lxr.free-electrons.com/source/drivers/clk/samsung/clk-exynos4.c#L276
>

Yes I noticed that the Exynos5420 driver also does the same but did not
want to do it there because I didn't know what value should had been used
for all the other clocks in the CLK_GATE_BUS_TOP register. But if I get
your explanation right, it is safe to do so since the register is going to
be reset to its default values anyways.

> Could this method work for your case as well? There would be no need
> to call any clock API at all, just low level register writes, which is
> okay, since this is a low level driver anyway.
>

Yes, the following patch [0] is enough to make S2R working. If you think
that is correct then I'll post it as a proper patch then.

> Best regards,
> Tomasz
> 

Best regards,
Javier

[0]