linux-kernel - Re: [PATCH 1/3] ARM: tegra: pcie: Add tegra3 support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130413102319.GA30758@avionic-0098.mockup.avionic-design.de>
Date:	Sat, 13 Apr 2013 12:23:19 +0200
From:	Thierry Reding <thierry.reding@...onic-design.de>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	Jay Agarwal <jagarwal@...dia.com>,
	Prashant Gaikwad <pgaikwad@...dia.com>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	Krishna Thota <kthota@...dia.com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Peter De Schrijver <pdeschrijver@...dia.com>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"olof@...om.net" <olof@...om.net>,
	Laxman Dewangan <ldewangan@...dia.com>,
	"bhelgaas@...gle.com" <bhelgaas@...gle.com>,
	"mturquette@...aro.org" <mturquette@...aro.org>,
	Juha Tukkinen <jtukkinen@...dia.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Hiroshi Doyu <hdoyu@...dia.com>
Subject: Re: [PATCH 1/3] ARM: tegra: pcie: Add tegra3 support

On Fri, Apr 12, 2013 at 09:34:13AM -0600, Stephen Warren wrote:
> 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.

That code is from tegra_pcie_power_off(), which is called only during
error cleanup or from tegra_pcie_put_resources() which in turn is also
only called in cleanup paths or during module/device removal. Disabling
as many regulators as possible is still what we want in that case, so
returning an error prematurely might leave more regulators turned on
than necessary.

Thierry

Content of type "application/pgp-signature" skipped