From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH] ARM: dts: Remove mau_pd node for Exynos5420 Date: Sat, 26 Apr 2014 17:18:20 +0200 Message-ID: <535BCE3C.7080502@gmail.com> References: <1398145155-4615-1-git-send-email-tushar.behera@linaro.org> <03b001cf5edc$ae42e770$0ac8b650$@samsung.com> <5358D46F.2050303@linaro.org> <5358E243.7010004@samsung.com> <5358EF70.7070809@linaro.org> <535AEFFE.7020900@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f44.google.com ([74.125.83.44]:57945 "EHLO mail-ee0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751243AbaDZPSX (ORCPT ); Sat, 26 Apr 2014 11:18:23 -0400 Received: by mail-ee0-f44.google.com with SMTP id e49so3520420eek.17 for ; Sat, 26 Apr 2014 08:18:21 -0700 (PDT) In-Reply-To: Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: Vikas Sajjan Cc: Tushar Behera , Tomasz Figa , Kukjin Kim , Alim Akhtar , linux-samsung-soc , jhbird.choi@samsung.com Hi Vikas, On 26.04.2014 13:57, Vikas Sajjan wrote: > Hi, > > > On Sat, Apr 26, 2014 at 5:00 AM, Tomasz Figa wrote: >> >> >> On 24.04.2014 13:03, Tushar Behera wrote: >>> >>> On 04/24/2014 03:36 PM, Tomasz Figa wrote: >>>> >>>> On 24.04.2014 11:07, Tushar Behera wrote: >>>>> >>>>> On 04/23/2014 03:43 PM, Kukjin Kim wrote: >>>>>> >>>>>> Tushar Behera wrote: >>>>>>> >>>>>>> >>>>>>> On 22 April 2014 13:08, Alim Akhtar wrote: >>>>>>>> >>>>>>>> Hi Tushar >>>>>>>> >>>>>>>> On Tue, Apr 22, 2014 at 11:09 AM, Tushar Behera >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> MAU powerdomain provides clocks for Audio sub-system block. This >>>>>>>>> block >>>>>>>>> comprises of the I2S audio controller, audio DMA blocks and Audio >>>>>>>>> sub-system clock registers. >>>>>>>>> >>>>>>>>> Right now, there is no way to hook up power-domains with clock >>>>>>> >>>>>>> providers. >>>>>>>>> >>>>>>>>> During late boot when this power-domain gets disabled, we get >>>>>>>>> following >>>>>>>>> external abort. >>>>>> >>>>>> >>>>>> + Jonghwan Choi >>>>>> >>>>>> Well, this is not a perfect solution to support MAU power domain, >>>>>> it's true it is a problem right now though. >>>>>> >>>>>> In other words, this is just temporal fix for the problem. >>>>>> >>>>>> How about accessing clock stuff for audio sub-system with handling >>>>>> MAU power domain via generic IO power domain? >>>>>> >>>>> >>>>> + Tomasz Figa >>>>> >>>>> Existing power domain driver exynos4_pm_init_power_domain is registered >>>>> with an arch_initcall whereas the clk-exynos-audss driver is registered >>>>> with core_initcall. Hence even if add mau_pd node to clk-exynos-audss >>>>> node, the binding with power-domain doesn't happen. >>>> >>>> >>>> I'd say core_initcall is way too early for clk-exynos-audss driver. It >>>> should be at most subsys_initcall. As far as I can see, all users of >>>> clocks provided by this driver (i.e. i2s) are probed at device_initcall >>>> level anyway. >>>> >>> >>> It is also used by ADMA node, which gets probed. >> >> >> If I'm looking correctly, ADMA is handled by pl330 driver which is >> registered at device_initcall level and so it shouldn't make problems with >> clk-exynos-audss driver being probed at subsys_initcall level. >> >> >>> >>>>> >>>>> Alternately, if Tomasz's patches are applied [1], power-domain binding >>>>> is successful. But because of the init order, clk-exynos-audss defers >>>>> probe resulting in a kernel crash. Forcing clk-exynos-audss to register >>>>> through arch_initcall() fixes this issue, but I am not sure if that is >>>>> okay. >>>> >>>> >>>> If the driver crashes on deferred probe, then it's a bug and it should >>>> be fixed. >>>> >>> >>> By the time clk-exynos-audss is getting called, mau_pd is already >>> disabled by power-domain driver. That is not getting enabled during >>> clk-exynos-audss probe. Am I missing something? >> >> >> Probably. The driver should enable runtime PM and call pm_runtime_get_sync() >> to make sure that power is supplied to the device it accesses. >> >> By the way, if defining MAU power domain in DT, then also all the devices >> inside of this domain should be bound to it, including ADMA and I2S, but I >> don't see neither of them having the "samsung,power-domain" property. >> > > According to UM of 5420, MAU has to be power gated is specific order > the SYS_PWR_CFG field of EXYNOS5_PAD_RETENTION_MAU_SYS_PWR_REG should > be set to 0, before actually power gating the MAU block. > I am NOT sure whether this is taken care in the mainline. > As per the current implementation in pm_domain.c, the > exynos_pd_power() just writes __raw_writel(pwr, base); > based on bool power_on passed. > We dont have the provision in the generic power domain framework to > have something like pre_power_off() or post_power_on(). Correct me if > am wrong. > > Even for power gating of ISP block a certain specific order needs to > be followed. > > So I think there is a need ops like pre_power_off(), post_power_on() > in generic power domain framework. > > let me know your opinion. I don't think there is any need to handle this in high level code. IMHO just extending Exynos power domain initialization code and exynos_pd_power() should be enough. Best regards, Tomasz