From patchwork Fri Jun 20 13:54:54 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nishanth Menon X-Patchwork-Id: 32284 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ie0-f197.google.com (mail-ie0-f197.google.com [209.85.223.197]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 55282203C2 for ; Fri, 20 Jun 2014 13:55:04 +0000 (UTC) Received: by mail-ie0-f197.google.com with SMTP id lx4sf25263067iec.8 for ; Fri, 20 Jun 2014 06:55:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:cc:subject:message-id :references:mime-version:in-reply-to:user-agent:sender:precedence :list-id:x-original-sender:x-original-authentication-results :mailing-list:list-post:list-help:list-archive:list-unsubscribe :content-type:content-disposition; bh=xR2egLHV7r+/FsYzsKHBfhofzvSuovsxV/V1mEc14P0=; b=k7wbx3WxwcmP257CXEBYw7mnH6I/CHniIwTaAJDPIJZf1I2Y00K+ol38HY+wtBsreJ QKhOah+9BypsroDXg2axSUQZjDSBjc8DlNMYGEHy3bLKGyh7evM/OqgkG9L8qTDSCL2V QqT8yUVDBM5anH2MRs0YWXeocQMN9xDfh1RKvkYlAkfUcWODGremHcYbu4caHESQn5h1 vy3SlapB071/aYRa/ZoYwpGouXJBh5yaBXtFD7Gc0bIBFLAm911HJHU6YUdr6bJr78yi YZZX1Ttfy/Ew9Trp67D0tJos4mvP+noKDTzSLxiyPhjYMD5DLGG2bwgEjDG6K3aN1itW RpOg== X-Gm-Message-State: ALoCoQk+BgJ1dLgp1Tq0wUt8zOsMsoWOMxpK7FHs95dupR3FrV0vOiYa1H17fXnFkxROiwsTuopm X-Received: by 10.182.60.40 with SMTP id e8mr1449609obr.7.1403272503667; Fri, 20 Jun 2014 06:55:03 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.19.173 with SMTP id 42ls982400qgh.27.gmail; Fri, 20 Jun 2014 06:55:03 -0700 (PDT) X-Received: by 10.58.152.234 with SMTP id vb10mr3162246veb.21.1403272503522; Fri, 20 Jun 2014 06:55:03 -0700 (PDT) Received: from mail-vc0-f182.google.com (mail-vc0-f182.google.com [209.85.220.182]) by mx.google.com with ESMTPS id i8si4059683vco.4.2014.06.20.06.55.03 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 20 Jun 2014 06:55:03 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.182 as permitted sender) client-ip=209.85.220.182; Received: by mail-vc0-f182.google.com with SMTP id il7so3471668vcb.13 for ; Fri, 20 Jun 2014 06:55:03 -0700 (PDT) X-Received: by 10.58.24.38 with SMTP id r6mr1164954vef.41.1403272503417; Fri, 20 Jun 2014 06:55:03 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp24186vcb; Fri, 20 Jun 2014 06:55:02 -0700 (PDT) X-Received: by 10.66.164.5 with SMTP id ym5mr5033548pab.50.1403272502555; Fri, 20 Jun 2014 06:55:02 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id hd9si5503884pac.147.2014.06.20.06.55.01; Fri, 20 Jun 2014 06:55:02 -0700 (PDT) Received-SPF: none (google.com: linux-kernel-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S967620AbaFTNzA (ORCPT + 14 others); Fri, 20 Jun 2014 09:55:00 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:54829 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966958AbaFTNy6 (ORCPT ); Fri, 20 Jun 2014 09:54:58 -0400 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id s5KDss26006431; Fri, 20 Jun 2014 08:54:54 -0500 Received: from DFLE73.ent.ti.com (dfle73.ent.ti.com [128.247.5.110]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id s5KDss4J011721; Fri, 20 Jun 2014 08:54:54 -0500 Received: from dflp32.itg.ti.com (10.64.6.15) by DFLE73.ent.ti.com (128.247.5.110) with Microsoft SMTP Server id 14.3.174.1; Fri, 20 Jun 2014 08:54:53 -0500 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id s5KDssQk020259; Fri, 20 Jun 2014 08:54:54 -0500 Date: Fri, 20 Jun 2014 08:54:54 -0500 From: Nishanth Menon To: Alexandre Courbot CC: Mark Brown , Stephen Warren , Keerthy , Thierry Reding , "linux-tegra@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Subject: Re: Palmas regulator broken (was Re: [PATCH] ARM: tegra: TN7: relax some regulators) Message-ID: <20140620135454.GA10566@kahuna> References: <1403164154-1362-1-git-send-email-acourbot@nvidia.com> <53A308C8.2070004@wwwdotorg.org> <20140619175643.GR5099@sirena.org.uk> <53A3C613.8030207@nvidia.com> <53A3D85E.7030704@nvidia.com> <20140620094119.GT5099@sirena.org.uk> <53A4039B.7000504@nvidia.com> <20140620132310.GA11936@kahuna> MIME-Version: 1.0 In-Reply-To: <20140620132310.GA11936@kahuna> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: nm@ti.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.182 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Content-Disposition: inline On 08:23-20140620, Nishanth Menon wrote: > + l-o, > http://marc.info/?t=140316427500004&r=1&w=2 full thread > > Minor change in subject to indicate palmas regulator fail > > On 18:49-20140620, Alexandre Courbot wrote: > > On 06/20/2014 06:41 PM, Mark Brown wrote: > > >* PGP Signed by an unknown key > > > > > >On Fri, Jun 20, 2014 at 03:44:46PM +0900, Alexandre Courbot wrote: > > > > > >>dbabd624d > > >>regulator: palmas: Reemove open coded functions with helper functions > > > > > >>Keerthy, Nishanth, could it be that there is still something wrong with the > > >>REGULATOR_LINEAR_RANGE() definitions? > > > > > >>This seems to be the cause for our trouble, but the other questions might > > >>still stand, in case there is interest in discussing them. > > > > > >There was a bug fix to the Palmas driver which just went to Linus the > > >other day, are you sure this isn't fixed in mainline (or -next, it's > > >been in -next for a week or something)? > > > > If you are talking about > > > > 6b7f2d82d5 > > regulator: palmas: Fix SMPS list for 0V > > > > then it is in my tree. There is actually no difference on > > palmas-regulator.c between my tree and the current -next (or Linus' > > tree for that instance). > > > > So it seems to be something else we are dealing with here. > > Your quote earlier in the thread > " > _regulator_is_enabled() *also* returns false > " > > Got me curious. Looking at the patch: > dbabd624d4eec50b623bab070d1e39a854b2d65c (regulator: palmas: Reemove > open coded functions with helper functions) > I noticed the following change > palmas_is_enabled_smps -> regulator_is_enabled_regmap > > So I decided to search for enable_reg in palmas-regulator.c and I think > it needs valid enable_reg, mask, value for regulator_is_enabled_regmap to work > :). > > Maybe to be sure, we could print the following: > PALMAS_SMPS8_VOLTAGE, PALMAS_SMPS8_CTRL, PALMAS_SMPS8_TSTEP, > > Anyways, I quickly boot tested the following on DRA7evm (which also uses Palmas): > [ 1.933939] palmas-pmic 48070000.i2c:tps659038@58:tps659038_pmic: enable_reg = 0x00, mask =0x00 > [ 1.944210] smps123: 850 <--> 1250 mV at 1060 mV > [ 1.950717] palmas-pmic 48070000.i2c:tps659038@58:tps659038_pmic: enable_reg = 0x00, mask =0x00 > [ 1.960754] smps45: 850 <--> 1150 mV at 1060 mV > [ 1.967048] palmas-pmic 48070000.i2c:tps659038@58:tps659038_pmic: enable_reg = 0x00, mask =0x00 > [ 1.977072] smps6: 850 <--> 1650 mV at 1060 mV > [ 1.983077] palmas-pmic 48070000.i2c:tps659038@58:tps659038_pmic: enable_reg = 0x00, mask =0x00 > [ 1.992994] smps7: 850 <--> 1030 mV at 1030 mV > [ 1.999238] palmas-pmic 48070000.i2c:tps659038@58:tps659038_pmic: enable_reg = 0x00, mask =0x00 > [ 2.009161] smps8: 850 <--> 1250 mV at 1060 mV > [ 2.015304] palmas-pmic 48070000.i2c:tps659038@58:tps659038_pmic: enable_reg = 0x00, mask =0x00 > > It does seem to me that either set_mode also should use core functions > OR you still need a palmas specific is_enable, enable/disable functions > (contrary to the claim of the patch in question - which I think > introduced regressions). > > Otherwise, completely untested diff below - can you give this a shot? > > diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c > index b982f0f..bbfe22f 100644 > --- a/drivers/regulator/palmas-regulator.c > +++ b/drivers/regulator/palmas-regulator.c > @@ -964,6 +964,20 @@ static int palmas_regulators_probe(struct platform_device *pdev) > return ret; > pmic->current_reg_mode[id] = reg & > PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK; > + > + dev_err(&pdev->dev, "enable_reg = 0x%02x, mask =0x%02x\n", > + pmic->desc[id].enable_reg, > + pmic->desc[id].enable_mask); > + pmic->desc[id].enable_reg = > + PALMAS_BASE_TO_REG(PALMAS_LDO_BASE, > + palmas_regs_info[id].ctrl_addr); > + pmic->desc[id].enable_mask = > + PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK; > + /* > + * The following completely ignores > + * pmic->current_reg_mode[id] (set_mode) > + */ > + pmic->desc[id].enable_val = SMPS_CTRL_MODE_ON; > } > > pmic->desc[id].type = REGULATOR_VOLTAGE; rev 2 of the diff - this does depened on the fact that regulator_desc is not memdup-ed by regulator code - that lets us do a bit of a trickery ;) - and I dropped the prints.. Unrelated: This makes me wonder why palmas_is_enabled_ldo at all? Keerthy, Mark, what do you think of the following (esp the flip of desc value): diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c index b982f0f..f01d9c5 100644 --- a/drivers/regulator/palmas-regulator.c +++ b/drivers/regulator/palmas-regulator.c @@ -299,7 +299,7 @@ static int palmas_set_mode_smps(struct regulator_dev *dev, unsigned int mode) struct palmas_pmic *pmic = rdev_get_drvdata(dev); int id = rdev_get_id(dev); unsigned int reg; - bool rail_enable = true; + bool rail_enable = true, enable_val = true; palmas_smps_read(pmic->palmas, palmas_regs_info[id].ctrl_addr, ®); reg &= ~PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK; @@ -318,6 +318,7 @@ static int palmas_set_mode_smps(struct regulator_dev *dev, unsigned int mode) reg |= SMPS_CTRL_MODE_PWM; break; default: + enable_val = false; return -EINVAL; } @@ -325,6 +326,11 @@ static int palmas_set_mode_smps(struct regulator_dev *dev, unsigned int mode) if (rail_enable) palmas_smps_write(pmic->palmas, palmas_regs_info[id].ctrl_addr, reg); + + /* Switch the enable value to ensure this is used for enable */ + if (enable_val) + pmic->desc[id].enable_val = pmic->current_reg_mode[id]; + return 0; } @@ -964,6 +970,14 @@ static int palmas_regulators_probe(struct platform_device *pdev) return ret; pmic->current_reg_mode[id] = reg & PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK; + + pmic->desc[id].enable_reg = + PALMAS_BASE_TO_REG(PALMAS_LDO_BASE, + palmas_regs_info[id].ctrl_addr); + pmic->desc[id].enable_mask = + PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK; + /* set_mode overrides this value */ + pmic->desc[id].enable_val = SMPS_CTRL_MODE_ON; } pmic->desc[id].type = REGULATOR_VOLTAGE;