Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751436AbbDBMW2 (ORCPT ); Thu, 2 Apr 2015 08:22:28 -0400 Received: from mail-qc0-f177.google.com ([209.85.216.177]:33130 "EHLO mail-qc0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750747AbbDBMWZ (ORCPT ); Thu, 2 Apr 2015 08:22:25 -0400 MIME-Version: 1.0 In-Reply-To: <551C71A5.1070903@collabora.co.uk> References: <1427730803-28635-1-git-send-email-javier.martinez@collabora.co.uk> <1427730803-28635-3-git-send-email-javier.martinez@collabora.co.uk> <551976F1.1000605@collabora.co.uk> <551AFCCE.4050404@collabora.co.uk> <551BD07E.2090506@samsung.com> <551BDA0A.7010704@collabora.co.uk> <551C2B6D.4010001@samsung.com> <551C71A5.1070903@collabora.co.uk> Date: Thu, 2 Apr 2015 17:52:24 +0530 Message-ID: Subject: Re: [RFC PATCH v3 2/2] clk: exynos5420: Make sure MDMA0 clock is enabled during suspend From: Abhilash Kesavan To: Javier Martinez Canillas Cc: Sylwester Nawrocki , Tomasz Figa , Stephen Boyd , Mike Turquette , Kukjin Kim , Olof Johansson , Doug Anderson , Krzysztof Kozlowski , Kevin Hilman , Tyler Baker , Chanwoo Choi , linux-arm-kernel , "linux-samsung-soc@vger.kernel.org" , linux-kernel Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5273 Lines: 127 Hi, On Thu, Apr 2, 2015 at 4:01 AM, Javier Martinez Canillas wrote: > Hello Sylwester, > > On 04/01/2015 07:31 PM, Sylwester Nawrocki wrote: >> On 01/04/15 13:44, Javier Martinez Canillas wrote: >>> On 04/01/2015 01:03 PM, Sylwester Nawrocki wrote: >>>> It's not clear what subsystems affect state of the CG_STATUSx registers, it >>>> would be good if we could get more information on that. They are in the PMU >>>> block and are related to LPI (Low Power Interface handshaking), but what >>>> subsystems/peripheral blocks exactly are associated with them it's not clear >>>> from the documentation. >>> >>> Yes, I've been looking at the docs again and found out a couple of things: >>> >>> * Each GC_STATUSx register bit is associated with an IP hw block >>> * Some LPI_MASKx registers maps exactly with the GC_STATUSx (i.e: 0 and 1) >>> and others maps only partially (i.e: LPI_MASK2 and GC_STATUS2) >> >> The CG_STATUSx and LPI_MASKx bits meaning is not matching according to >> documentation I have. I guess you've got something newer than REV0.00? >> > > My Exynos5420 UM is Revision 1.00 dated February 2014 and GC_STATUS0 bits > maps LPI_MASK0 with the exception of bit 16 (NR3D) that is not mentioned > in GC_STATUS0, there is a hole between 15 (DIS) and 17 (FIMC_SCALERP). > > GC_STATUS1 maps exactly with LPI_MASK1 and GC_STATUS2 and LPI_MASK2 have > the same bits from 0 to 5 and then differ from there. > >>> So it is related to LPI as you said and both LPI_MASKx and GC_STATUSx are >>> part of the PMU register address space. >>> >>> In the particular case of aclk266_g2d, the doc says that the clock can only >>> be gated when CG_STATUS0[20] and CG_STATUS0[21] are 0. These are associated >>> with the SSS and SSS_SLIM respectively which AFAIU are crypto h/w modules. >> >> In my Exynos5420 UM ACLK_266_G2D is associated with CG_STATUS0 register >> bits 22, 21, which in turn correspond to NR3D and DIS IP blocks, i.e. >> the camera subsystem. Such a dependency would be rather surprising. >> > > Sorry, it was a typo error and I actually meant CG_STATUS0 21 and 22 but > these correspond in the documentation I've to 21 (SSS) and 22 (SSS_SLIM). > > As I mentioned before, DIS correspond to CG_STATUS0 15 and NR3D doesn't > have a corresponding bit in CG_STATUS0. But I don't know if that is an > error in the doc I've since is suspicious that is the only difference > between LPI_MASK0 and CG_STATUS0. > >>>> I think it's essential to understand what triggers changes in CG_STATUSx >>>> registers, before we start checking their value in the clock driver. >>>> >>> >>> Indeed, we should really understand what the status on these registers >>> means. Also is not clear from the docs how much time should be waited, >>> how long until giving up, etc. >> >> Exactly, I checked some kernels from http://opensource.samsung.com >> (e.g. SM-N900_JB_Opensource.zip) for CG_STATUSx, but I didn't find anything >> related to these registers yet, except the address macro definitions >> and debug traces in the power domains driver. >> > > Yes, I also looked in the ChromiumOS v3.8 kernel but didn't find anything. > >>>> Also it might be that there are indeed some clocks which must stay enabled >>>> over suspend/resume cycle, then the approach with enabling/disabling clocks >>>> in the clock driver might not be such a hack as it looks at first sight. >>>> >>> >>> Having a clock driver to both a provider and consumer feels hacky to me as >>> well but I didn't find a better way to solve this issue... another option >>> is to have this workaround to solve the S2R issue while we figure out what >>> the the state in the CG_STATUSx really mean. >> >> Let's try to diagnose the issue best we can, then we would choose the most >> accurate bug fix. >> > > Sounds good to me. Based on the earlier comments I was trying to isolate if: 1) s2r fails because we gate aclk266_g2d (but it is one of those clocks that needs to be always on prior to suspend). 2) s2r fails because we gate aclk266_g2d when CG_STATUS0[21:20] bits are not 0 (thus not following the spec). As I did not have access to the hardware guys who could possibly confirm 1), I decided to a) find a configuration where CG_STATUS0 allows gating of the aclk266_g2d clock (i.e. CG_STATUS0[22:21] are 0). b) disable the aclk266_g2d clock using such a configuration. c) check s2r. I found a configuration [1] which gave the following after boot-up: # devmem 0x10040914 0xFD800014 (CG_STATUS0[22:21] is 0) # devmem 0x10020700 0xC6F8DE9F (aclk266_g2d is enabled) At this point s2r works. I rebooted the board with the same config as above and then disabled aclk266_g2d. # devmem 0x10020700 32 0xC6F8DE9D # devmem 0x10020700 0xC6F8DE9D (aclk266_g2d is disabled) # devmem 0x10040914 0xFD800014 and tried s2r - It fails. >From the results, disabling the clock seems to cause the issue rather than the CG_STATUS violation. This is all a little confusing, so please let me know if I have missed something. Regards, Abhilash -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/