From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934233AbbCPPMv (ORCPT ); Mon, 16 Mar 2015 11:12:51 -0400 Received: from mail-ig0-f174.google.com ([209.85.213.174]:37692 "EHLO mail-ig0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932191AbbCPPMo (ORCPT ); Mon, 16 Mar 2015 11:12:44 -0400 MIME-Version: 1.0 In-Reply-To: References: <1426112117-18220-1-git-send-email-dianders@chromium.org> <1426112117-18220-2-git-send-email-dianders@chromium.org> Date: Mon, 16 Mar 2015 08:12:43 -0700 X-Google-Sender-Auth: OpVdmMuujOD3NCIM0CgsATRBCSM Message-ID: Subject: Re: [PATCH v4 2/4] mmc: core: Add mmc_regulator_set_vqmmc() From: Doug Anderson To: Ulf Hansson Cc: Heiko Stuebner , Jaehoon Chung , Seungwon Jeon , Mark Brown , Alexandru Stan , Alim Akhtar , Sonny Rao , Andrew Bresticker , Addy Ke , Javier Martinez Canillas , "open list:ARM/Rockchip SoC..." , "linux-arm-kernel@lists.infradead.org" , Chris Ball , Johan Rudholm , Adrian Hunter , Tim Kryger , Andrew Gabbasov , Sascha Hauer , linux-mmc , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Ulf, On Mon, Mar 16, 2015 at 7:05 AM, Ulf Hansson wrote: >> + switch (ios->signal_voltage) { >> + case MMC_SIGNAL_VOLTAGE_120: >> + return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, >> + 1200000, 100000); > > Is 1V the lowest possible value? How did you get to that? I think you've added a zero in your mind and not realized that I'm calling regulator_set_voltage_tol() here and in other calls. Please read the above as: * Try to set the voltage to exactly 1,200,000 uV (1.2V). * If you can't get 1.2V exactly, a tolerance ("tol") of 100,000 uV (.1V) is OK. * In other words, 1.1V - 1.3V are OK, but aim for 1.2V >> + case MMC_SIGNAL_VOLTAGE_180: >> + return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, >> + 1800000, 100000); > > Is 1V the lowest possible value? How did you get to that? Again, check my zeros. This should be 1.7 - 1.9V, aiming for 1.8V. >> + case MMC_SIGNAL_VOLTAGE_330: >> + return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, >> + regulator_get_voltage(mmc->supply.vmmc), 300000); > > Why 3V? Shouldn't it be 2.7V? How will else those SoC that for example > supports 2.9V only work? This will get us within .3V of whatever vmmc is. If vmmc is 3.3V, it will allow vqmmc of 3.0V - 3.6V. This _seems_ sane to me and given any sane system design we should be fine here, I think. I can't see someone designing a system where vqmmc was not within .3V of vmmc, can you? If we think someone will actually build a system where vmmc is 3.3V and vqmmc can't go higher than 2.7V then we'll either need to increase the tolerance here or add a new asymmetric system call like my original patches did. >> int mmc_regulator_get_supply(struct mmc_host *mmc); > > One more thought,s as for the vmmc regulator we have a > "regulator_enabled" member in the mmc_host. Should we add a similar > member for vqmmc? That would prevent host drivers from keeping track > of this state themselves. Yeah, that does sound nice. Are you suggesting that I modify this patch or submit a new one. Let me know. -Doug