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: <51682975.1080302@wwwdotorg.org>
Date:	Fri, 12 Apr 2013 09:34:13 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Jay Agarwal <jagarwal@...dia.com>
CC:	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>,
	"thierry.reding@...onic-design.de" <thierry.reding@...onic-design.de>,
	"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 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/