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

[Stephen Warren <swarren@wwwdotorg.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.