From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Subject: Re: [PATCH 1/3] ARM: tegra: pcie: Add tegra3 support Date: Fri, 12 Apr 2013 09:34:13 -0600 Message-ID: <51682975.1080302@wwwdotorg.org> References: <1365435688-4179-1-git-send-email-jagarwal@nvidia.com> <51630852.1000107@wwwdotorg.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-pci-owner@vger.kernel.org To: Jay Agarwal Cc: Prashant Gaikwad , "linux@arm.linux.org.uk" , Krishna Thota , "linux-pci@vger.kernel.org" , Peter De Schrijver , "thierry.reding@avionic-design.de" , "linux-tegra@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "olof@lixom.net" , Laxman Dewangan , "bhelgaas@google.com" , "mturquette@linaro.org" , Juha Tukkinen , "linux-arm-kernel@lists.infradead.org" , Hiroshi Doyu List-Id: linux-tegra@vger.kernel.org On 04/12/2013 08:58 AM, Jay Agarwal wrote: >>> err = regulator_disable(pcie->pex_clk_supply); >>> if (err < 0) >>> - dev_err(pcie->dev, "failed to disable pex-clk regulator: >> %d\n", >>> + dev_warn(pcie->dev, "failed to disable pex-clk regulator: >> %d\n", >>> err); >>> >>> err = regulator_disable(pcie->vdd_supply); >>> if (err < 0) >>> - dev_err(pcie->dev, "failed to disable VDD regulator: %d\n", >>> + dev_warn(pcie->dev, "failed to disable VDD regulator: >> %d\n", >>> err); >> >> Please explain why that change is correct. If the regulators only exist on >> Tegra20, please represent that fact in the SoC data. Regulators must always >> exist, so enable/disable should never fail due to missing regulators. Actual >> run-time failures seem like something that really is an error. >> > [>] These regulators are needed for both tegra20 & tegra30. Since we are not returning error here, so changed to dev_warn. If the regulators are required, then any failure to operate them should be an error, hence dev_err() seems correct. As to why the code doesn't actually return an error? I'm not sure. Perhaps that should be fixed with a separate patch, although I don't recall exactly where in the code the above excerpt is; if it's in remove(), then continuing on without returning an error would be appropriate.