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-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html