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

From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Kevin Hilman <khilman@kernel.org>
Cc: Abhilash Kesavan <kesavan.abhilash@gmail.com>,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>,
	"linux-samsung-soc@vger.kernel.org" 
	<linux-samsung-soc@vger.kernel.org>,
	Mike Turquette <mturquette@linaro.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Tyler Baker <tyler.baker@linaro.org>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Doug Anderson <dianders@chromium.org>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Kukjin Kim <kgene@kernel.org>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Olof Johansson <olof@lixom.net>,
	Javier Martinez Canillas <javier.martinez@collabora.co.uk>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH v3 2/2] clk: exynos5420: Make sure MDMA0 clock is enabled during suspend
Date: Wed, 1 Apr 2015 11:16:08 +0200	[thread overview]
Message-ID: <CAJKOXPftzX4KT=RNVnuD3UKOLXKkKr0b9i9_HDE47=8z9AyzUQ@mail.gmail.com> (raw)
In-Reply-To: <7h1tk47awc.fsf@deeprootsystems.com>

2015-04-01 6:03 GMT+02:00 Kevin Hilman <khilman@kernel.org>:
> Abhilash Kesavan <kesavan.abhilash@gmail.com> writes:
>
>> On Wed, Apr 1, 2015 at 2:32 AM, Kevin Hilman <khilman@kernel.org> wrote:
>>> Javier Martinez Canillas <javier.martinez@collabora.co.uk> writes:
>>>
>>> [...]
>>>
>>>> Unfortunately I don't fully understand why this clock needs to be
>>>> enabled. It would be good if someone at Samsung can explain in more
>>>> detail what the real problem really is.
>>>
>>> +1
>>>
>>> Maybe Abhilash can shed some light here?
>>>
>>> We really should know *why* this is needed because having the fix in the
>>> clock driver just doesn't seem right.  It seems like the DMA driver
>>> should be managing this clock.
>>
>> I think my last mail might not have reached you (was accidentally sent
>> as html).
>
> Yeah, I saw it a bit later in Javier's reply.  Thanks for doing the
> research and reporting back.
>
>> We are gating the aclk266_g2d clock without checking the
>> CG_STATUS0 register bits as specified in the UM. It looks like we need
>> to keep several clocks alive or gate them only after checking the
>> CG_STATUSx register bits.
>
> I dont' know much about this clock hardware, but to me it sounds like a
> clock driver bug.  The suspend fix Javier is proposing would fix it, but
> to me it sounds like the clock driver needs to actually start checking
> these CG_STATUSx bits before gating clocks.
>
> Otherwise, we might fix this current bug but a similar one will come and
> bite us another day.

Actually it looks kind a similar to issue with adma/mau_epll clocks:
https://lkml.org/lkml/2014/12/5/291

In that case runtime PM for pl330 caused adma clock to be gated. This
lead to gating its parent clock - mau_epll. The clock hierarchy is
strange for maudio domain. Basically mau_epll is needed to access
maudio clocks however these clocks are not children of mau_epll.

I think we should not enable the mdma0 but instead we should find the
proper parent which needs to be enabled.

Anyway it is kind of annoying (or funny if one has sense of black
humour) that two issues are exposed by runtime PM for pl330 driver.

Best regards,
Krzysztof