Commit 962399bb authored by Mark Brown's avatar Mark Brown Committed by Jens Axboe

ata: libahci_platform: Fix regulator_get_optional() misuse

This driver is using regulator_get_optional() to handle all the supplies
that it handles, and only ever enables and disables all supplies en masse
without ever doing any other configuration of the device to handle missing
power. These are clear signs that the API is being misused - it should only
be used for supplies that may be physically absent from the system and in
these cases the hardware usually needs different configuration if the
supply is missing. Instead use normal regualtor_get(), if the supply is
not described in DT then the framework will substitute a dummy regulator in
so no special handling is needed by the consumer driver.

In the case of the PHY regulator the handling in the driver is a hack to
deal with integrated PHYs; the supplies are only optional in the sense
that that there's some confusion in the code about where they're bound to.
From a code point of view they function exactly as normal supplies so can
be treated as such. It'd probably be better to model this by instantiating
a PHY object for integrated PHYs.
Reviewed-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarMark Brown <broonie@kernel.org>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 7ce23e8e
...@@ -153,17 +153,13 @@ int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv) ...@@ -153,17 +153,13 @@ int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv)
{ {
int rc, i; int rc, i;
if (hpriv->ahci_regulator) { rc = regulator_enable(hpriv->ahci_regulator);
rc = regulator_enable(hpriv->ahci_regulator); if (rc)
if (rc) return rc;
return rc;
}
if (hpriv->phy_regulator) { rc = regulator_enable(hpriv->phy_regulator);
rc = regulator_enable(hpriv->phy_regulator); if (rc)
if (rc) goto disable_ahci_pwrs;
goto disable_ahci_pwrs;
}
for (i = 0; i < hpriv->nports; i++) { for (i = 0; i < hpriv->nports; i++) {
if (!hpriv->target_pwrs[i]) if (!hpriv->target_pwrs[i])
...@@ -181,11 +177,9 @@ int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv) ...@@ -181,11 +177,9 @@ int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv)
if (hpriv->target_pwrs[i]) if (hpriv->target_pwrs[i])
regulator_disable(hpriv->target_pwrs[i]); regulator_disable(hpriv->target_pwrs[i]);
if (hpriv->phy_regulator) regulator_disable(hpriv->phy_regulator);
regulator_disable(hpriv->phy_regulator);
disable_ahci_pwrs: disable_ahci_pwrs:
if (hpriv->ahci_regulator) regulator_disable(hpriv->ahci_regulator);
regulator_disable(hpriv->ahci_regulator);
return rc; return rc;
} }
EXPORT_SYMBOL_GPL(ahci_platform_enable_regulators); EXPORT_SYMBOL_GPL(ahci_platform_enable_regulators);
...@@ -207,10 +201,8 @@ void ahci_platform_disable_regulators(struct ahci_host_priv *hpriv) ...@@ -207,10 +201,8 @@ void ahci_platform_disable_regulators(struct ahci_host_priv *hpriv)
regulator_disable(hpriv->target_pwrs[i]); regulator_disable(hpriv->target_pwrs[i]);
} }
if (hpriv->ahci_regulator) regulator_disable(hpriv->ahci_regulator);
regulator_disable(hpriv->ahci_regulator); regulator_disable(hpriv->phy_regulator);
if (hpriv->phy_regulator)
regulator_disable(hpriv->phy_regulator);
} }
EXPORT_SYMBOL_GPL(ahci_platform_disable_regulators); EXPORT_SYMBOL_GPL(ahci_platform_disable_regulators);
/** /**
...@@ -359,7 +351,7 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port, ...@@ -359,7 +351,7 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,
struct regulator *target_pwr; struct regulator *target_pwr;
int rc = 0; int rc = 0;
target_pwr = regulator_get_optional(dev, "target"); target_pwr = regulator_get(dev, "target");
if (!IS_ERR(target_pwr)) if (!IS_ERR(target_pwr))
hpriv->target_pwrs[port] = target_pwr; hpriv->target_pwrs[port] = target_pwr;
...@@ -436,16 +428,14 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, ...@@ -436,16 +428,14 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
hpriv->clks[i] = clk; hpriv->clks[i] = clk;
} }
hpriv->ahci_regulator = devm_regulator_get_optional(dev, "ahci"); hpriv->ahci_regulator = devm_regulator_get(dev, "ahci");
if (IS_ERR(hpriv->ahci_regulator)) { if (IS_ERR(hpriv->ahci_regulator)) {
rc = PTR_ERR(hpriv->ahci_regulator); rc = PTR_ERR(hpriv->ahci_regulator);
if (rc == -EPROBE_DEFER) if (rc != 0)
goto err_out; goto err_out;
rc = 0;
hpriv->ahci_regulator = NULL;
} }
hpriv->phy_regulator = devm_regulator_get_optional(dev, "phy"); hpriv->phy_regulator = devm_regulator_get(dev, "phy");
if (IS_ERR(hpriv->phy_regulator)) { if (IS_ERR(hpriv->phy_regulator)) {
rc = PTR_ERR(hpriv->phy_regulator); rc = PTR_ERR(hpriv->phy_regulator);
if (rc == -EPROBE_DEFER) if (rc == -EPROBE_DEFER)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment