From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752001AbcIBCoL (ORCPT ); Thu, 1 Sep 2016 22:44:11 -0400 Received: from regular1.263xmail.com ([211.150.99.130]:54829 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751029AbcIBCoJ (ORCPT ); Thu, 1 Sep 2016 22:44:09 -0400 X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-ADDR-CHECKED: 0 X-RL-SENDER: xzy.xu@rock-chips.com X-FST-TO: wxt@rock-chips.com X-SENDER-IP: 103.29.142.67 X-LOGIN-NAME: xzy.xu@rock-chips.com X-UNIQUE-TAG: <626744902ce5d31c48bd7c09eac9b66c> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399 To: Doug Anderson References: <20160827134103.28160-1-xzy.xu@rock-chips.com> <20160827134103.28160-3-xzy.xu@rock-chips.com> <2ab8ab94-fa4d-2cd6-5805-a92ac5f9697e@rock-chips.com> <57C3A30B.5080707@rock-chips.com> <77f5cff1-80a2-76b3-40e9-f77caced2257@rock-chips.com> <411dfc7b-1089-61a8-7a7e-6f378c0a6074@rock-chips.com> Cc: Mark Rutland , "devicetree@vger.kernel.org" , Ulf Hansson , Brian Norris , =?UTF-8?Q?Heiko_St=c3=bcbner?= , Xing Zheng , Masahiro Yamada , Frank Wang , Catalin Marinas , Shawn Lin , Elaine Zhang , Will Deacon , "linux-kernel@vger.kernel.org" , "open list:ARM/Rockchip SoC..." , Rob Herring , David Wu , Shunqian Zheng , Jianqun Xu , "linux-arm-kernel@lists.infradead.org" , Caesar Wang From: Ziyuan Xu Message-ID: <16314a2c-1ed9-0307-3497-9c1d8d41a149@rock-chips.com> Date: Fri, 2 Sep 2016 10:35:54 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年09月02日 05:29, Doug Anderson wrote: > Hi, > > On Wed, Aug 31, 2016 at 11:56 PM, Ziyuan Xu wrote: >> Hi >> >> >> On 2016年09月01日 12:20, Doug Anderson wrote: >>> Hi, >>> >>> On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu wrote: >>>>> This is fine to pick up _only_ if you don't care about suspend/resume. >>>>> If you care about suspend/resume then someone needs to first write a >>>>> patch that will re-init all "corecfg" values after power is turned on. >>>> >>>> Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we >>>> don't need to strore/re-init it after resume. >>>> corecfg_clockmultiplier is only used to fetch host->clk_mul, and >>>> host->clk_mul has been a fixed value at run-time, unless driver unbind. >>>> The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to check >>>> the xin_clk at probe time, we don't reference it at run-time. >>>> BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC >>>> works >>>> fine. >>> I guess I don't actually know how the corecfg_clockmultiplier and >>> corecfg_baseclkfreq fields are actually used, but I presume that they >>> actually do something useful and aren't used to just communicate back >>> to software? >> >> Take corecfg_clockmultiplier as example. >> 1. sdhci driver fetch host->clk_mul from corecfg_clockmultiplier >> 2. mmc->f_min and mmc->f_max are calculated via host->clk_mul, they're used >> for further initialization. >> 3. if the corecfg_clockmultiplier is incorrect, sdhci will use improper >> frequency to play. >> >> I think we don't need to store it due to it's a fixed value at run-time, >> even if it is reset after a power cycle, the above will not be changed via >> software, except for dirver unbind . >> >>> I know that: >>> >>> 1. If I don't pick this patch and I suspend/resume, >>> corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after >>> suspend / resume. >>> >>> 2. If I do pick this patch and I suspend/resume, >>> corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after >>> suspend/resume (tested by reading /dev/mem directly from userspace >>> after suspend/resume). >>> >>> >>> Are you saying that it is unimportant that corecfg_clockmultiplier and >>> corecfg_baseclkfreq are wrong? >> >> Yup, corecfg_* stuff will be reset after a power cycle. >> I mean that we need only to guarantee they're correct at probe time. > So are you saying that the entire purpose of "corecfg_clockmultiplier" > is that causes the "ClockMultiplier" field of the "EMMCCORE_CAP" > register to get a certain value? > ...and that the entire purpose of "corecfg_baseclkfreq" is that it > causes the "BaseClockFreqSDClock" field of the "EMMCCORE_CAP" register > to get a certain value? Yes, on rk3399: corecfg_clockmultiplier <===> EMMCCORE_CAP1[23:16] ClockMultiplier corecfg_baseclkfreq <===> EMMCCORE_CAP[15:8] BaseClockFreqSDClock If you re-write to either corecfg_* stuff, the corresponding CAP register field will be changed too. sdhci driver will fetch CAP register for initialization, we only need to guarantee they're correct at probe time. Did that all make sense? > > That would have been nice to know before. I had assumed that those > "corecfg" settings did something else more useful. > > If it is indeed true that these corecfg values are as silly as it > seems, then I guess it's not terribly important to re-set them after > suspend/resume. Eventually it would be nice/clean to actually do so > (in case the SDHCI driver eventually changes), but I guess we wouldn't > need to block. this patch from landing. > > Can you please confirm my understanding above? > > > -Doug > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip