summaryrefslogtreecommitdiff
path: root/drivers/ata
diff options
context:
space:
mode:
authorMark Brown <broonie@kernel.org>2019-10-16 13:51:05 +0300
committerJens Axboe <axboe@kernel.dk>2019-10-25 23:22:20 +0300
commit962399bb7fbf5ce0c5b768ca7115614f31ff8f3f (patch)
tree14f1f47db642455f58f3dc3dfc1817b7630d76d8 /drivers/ata
parent7ce23e8e0a9cd38338fc8316ac5772666b565ca9 (diff)
downloadlinux-962399bb7fbf5ce0c5b768ca7115614f31ff8f3f.tar.xz
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: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Jens Axboe <axboe@kernel.dk>
Diffstat (limited to 'drivers/ata')
-rw-r--r--drivers/ata/libahci_platform.c38
1 files changed, 14 insertions, 24 deletions
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index e742780950de..8befce036af8 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -153,17 +153,13 @@ int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv)
{
int rc, i;
- if (hpriv->ahci_regulator) {
- rc = regulator_enable(hpriv->ahci_regulator);
- if (rc)
- return rc;
- }
+ rc = regulator_enable(hpriv->ahci_regulator);
+ if (rc)
+ return rc;
- if (hpriv->phy_regulator) {
- rc = regulator_enable(hpriv->phy_regulator);
- if (rc)
- goto disable_ahci_pwrs;
- }
+ rc = regulator_enable(hpriv->phy_regulator);
+ if (rc)
+ goto disable_ahci_pwrs;
for (i = 0; i < hpriv->nports; i++) {
if (!hpriv->target_pwrs[i])
@@ -181,11 +177,9 @@ disable_target_pwrs:
if (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:
- if (hpriv->ahci_regulator)
- regulator_disable(hpriv->ahci_regulator);
+ regulator_disable(hpriv->ahci_regulator);
return rc;
}
EXPORT_SYMBOL_GPL(ahci_platform_enable_regulators);
@@ -207,10 +201,8 @@ void ahci_platform_disable_regulators(struct ahci_host_priv *hpriv)
regulator_disable(hpriv->target_pwrs[i]);
}
- if (hpriv->ahci_regulator)
- regulator_disable(hpriv->ahci_regulator);
- if (hpriv->phy_regulator)
- regulator_disable(hpriv->phy_regulator);
+ regulator_disable(hpriv->ahci_regulator);
+ regulator_disable(hpriv->phy_regulator);
}
EXPORT_SYMBOL_GPL(ahci_platform_disable_regulators);
/**
@@ -359,7 +351,7 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,
struct regulator *target_pwr;
int rc = 0;
- target_pwr = regulator_get_optional(dev, "target");
+ target_pwr = regulator_get(dev, "target");
if (!IS_ERR(target_pwr))
hpriv->target_pwrs[port] = target_pwr;
@@ -436,16 +428,14 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
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)) {
rc = PTR_ERR(hpriv->ahci_regulator);
- if (rc == -EPROBE_DEFER)
+ if (rc != 0)
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)) {
rc = PTR_ERR(hpriv->phy_regulator);
if (rc == -EPROBE_DEFER)