Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424481AbdDUTDZ (ORCPT ); Fri, 21 Apr 2017 15:03:25 -0400 Received: from mail.kernel.org ([198.145.29.136]:34380 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1424291AbdDUTDW (ORCPT ); Fri, 21 Apr 2017 15:03:22 -0400 Date: Fri, 21 Apr 2017 14:03:18 -0500 From: Bjorn Helgaas To: Brian Norris Cc: Bjorn Helgaas , linux-kernel@vger.kernel.org, Shawn Lin , Jeffy Chen , Wenrui Li , linux-pci@vger.kernel.org, linux-rockchip@lists.infradead.org Subject: Re: [PATCH v2 1/5] PCI: rockchip: fix sign issues for current limits Message-ID: <20170421190318.GA2638@bhelgaas-glaptop.roam.corp.google.com> References: <20170310024617.67303-1-briannorris@chromium.org> <20170323222717.GD23612@bhelgaas-glaptop.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170323222717.GD23612@bhelgaas-glaptop.roam.corp.google.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3845 Lines: 106 On Thu, Mar 23, 2017 at 05:27:17PM -0500, Bjorn Helgaas wrote: > On Thu, Mar 09, 2017 at 06:46:13PM -0800, Brian Norris wrote: > > The regulator framework can return negative error codes via > > regulator_get_current_limit() for regulators that don't provide current > > information. The subsequent check for postive values isn't very useful, > > if the variable is unsigned. > > > > Let's just match the signedness of the return value. > > > > Prevents error messages like this, seen on Samsung Chromebook Plus: > > > > [ 1.069372] rockchip-pcie f8000000.pcie: invalid power supply > > > > Fixes: 4816c4c7b82b ("PCI: rockchip: Provide captured slot power limit and scale") > > Signed-off-by: Brian Norris > > Acked-by: Shawn Lin > > I applied the first two patches (this already has Shawn's ack and the > second is trivially obvious) to pci/host-rockchip. I'm not sure what the > current state of the others is. I applied patches 3-5 with Shawn's ack to pci/host-rockchip for v4.12, thanks! > > --- > > v2: add Shawn's ack > > --- > > drivers/pci/host/pcie-rockchip.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c > > index 26ddd3535272..d785f64ec03b 100644 > > --- a/drivers/pci/host/pcie-rockchip.c > > +++ b/drivers/pci/host/pcie-rockchip.c > > @@ -425,7 +425,8 @@ static struct pci_ops rockchip_pcie_ops = { > > > > static void rockchip_pcie_set_power_limit(struct rockchip_pcie *rockchip) > > { > > - u32 status, curr, scale, power; > > + int curr; > > + u32 status, scale, power; > > > > if (IS_ERR(rockchip->vpcie3v3)) > > return; > > -- > > 2.12.0.246.ga2ecc84866-goog > > > > commit 73edd2b180a18024605c599074596a9e22d745d6 > Author: Bjorn Helgaas > Date: Thu Mar 23 17:21:26 2017 -0500 > > PCI: rockchip: Unindent rockchip_pcie_set_power_limit() > > If regulator_get_current_limit() returns 0 or error, return early so the > body of the function doesn't have to be indented as the body of an "if" > statement. No functional change intended. > > Signed-off-by: Bjorn Helgaas > > diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c > index d785f64ec03b..7f76ff6af0ba 100644 > --- a/drivers/pci/host/pcie-rockchip.c > +++ b/drivers/pci/host/pcie-rockchip.c > @@ -438,24 +438,25 @@ static void rockchip_pcie_set_power_limit(struct rockchip_pcie *rockchip) > * to the actual power supply. > */ > curr = regulator_get_current_limit(rockchip->vpcie3v3); > - if (curr > 0) { > - scale = 3; /* 0.001x */ > - curr = curr / 1000; /* convert to mA */ > - power = (curr * 3300) / 1000; /* milliwatt */ > - while (power > PCIE_RC_CONFIG_DCR_CSPL_LIMIT) { > - if (!scale) { > - dev_warn(rockchip->dev, "invalid power supply\n"); > - return; > - } > - scale--; > - power = power / 10; > - } > + if (curr <= 0) > + return; > > - status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_DCR); > - status |= (power << PCIE_RC_CONFIG_DCR_CSPL_SHIFT) | > - (scale << PCIE_RC_CONFIG_DCR_CPLS_SHIFT); > - rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_DCR); > + scale = 3; /* 0.001x */ > + curr = curr / 1000; /* convert to mA */ > + power = (curr * 3300) / 1000; /* milliwatt */ > + while (power > PCIE_RC_CONFIG_DCR_CSPL_LIMIT) { > + if (!scale) { > + dev_warn(rockchip->dev, "invalid power supply\n"); > + return; > + } > + scale--; > + power = power / 10; > } > + > + status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_DCR); > + status |= (power << PCIE_RC_CONFIG_DCR_CSPL_SHIFT) | > + (scale << PCIE_RC_CONFIG_DCR_CPLS_SHIFT); > + rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_DCR); > } > > /**