[PATCH] net: ks8851: Don't use regulator_get_optional()

[PATCH] net: ks8851: Don't use regulator_get_optional()

[Stephen Boyd]

We shouldn't be using regulator_get_optional() here. These
regulators are always present as part of the physical design and
there isn't any way to use an internal regulator or change the
source of the reference voltage via software. Given that the only
users of this driver in the kernel are DT based, this change
should be transparent to them even if they don't specify any
supplies because the regulator framework will insert dummy
supplies as needed.

Cc: Nishanth Menon <nm@ti.com>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/net/ethernet/micrel/ks8851.c | 50 ++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index e72918970a58..66d4ab703f45 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -1441,32 +1441,30 @@ static int ks8851_probe(struct spi_device *spi)
 		}
 	}
 
-	ks->vdd_io = devm_regulator_get_optional(&spi->dev, "vdd-io");
+	ks->vdd_io = devm_regulator_get(&spi->dev, "vdd-io");
 	if (IS_ERR(ks->vdd_io)) {
 		ret = PTR_ERR(ks->vdd_io);
-		if (ret == -EPROBE_DEFER)
-			goto err_reg_io;
-	} else {
-		ret = regulator_enable(ks->vdd_io);
-		if (ret) {
-			dev_err(&spi->dev, "regulator vdd_io enable fail: %d\n",
-				ret);
-			goto err_reg_io;
-		}
+		goto err_reg_io;
+	}
+
+	ret = regulator_enable(ks->vdd_io);
+	if (ret) {
+		dev_err(&spi->dev, "regulator vdd_io enable fail: %d\n",
+			ret);
+		goto err_reg_io;
 	}
 
-	ks->vdd_reg = devm_regulator_get_optional(&spi->dev, "vdd");
+	ks->vdd_reg = devm_regulator_get(&spi->dev, "vdd");
 	if (IS_ERR(ks->vdd_reg)) {
 		ret = PTR_ERR(ks->vdd_reg);
-		if (ret == -EPROBE_DEFER)
-			goto err_reg;
-	} else {
-		ret = regulator_enable(ks->vdd_reg);
-		if (ret) {
-			dev_err(&spi->dev, "regulator vdd enable fail: %d\n",
-				ret);
-			goto err_reg;
-		}
+		goto err_reg;
+	}
+
+	ret = regulator_enable(ks->vdd_reg);
+	if (ret) {
+		dev_err(&spi->dev, "regulator vdd enable fail: %d\n",
+			ret);
+		goto err_reg;
 	}
 
 	if (gpio_is_valid(gpio)) {
@@ -1572,11 +1570,9 @@ err_irq:
 	if (gpio_is_valid(gpio))
 		gpio_set_value(gpio, 0);
 err_id:
-	if (!IS_ERR(ks->vdd_reg))
-		regulator_disable(ks->vdd_reg);
+	regulator_disable(ks->vdd_reg);
 err_reg:
-	if (!IS_ERR(ks->vdd_io))
-		regulator_disable(ks->vdd_io);
+	regulator_disable(ks->vdd_io);
 err_reg_io:
 err_gpio:
 	free_netdev(ndev);
@@ -1594,10 +1590,8 @@ static int ks8851_remove(struct spi_device *spi)
 	free_irq(spi->irq, priv);
 	if (gpio_is_valid(priv->gpio))
 		gpio_set_value(priv->gpio, 0);
-	if (!IS_ERR(priv->vdd_reg))
-		regulator_disable(priv->vdd_reg);
-	if (!IS_ERR(priv->vdd_io))
-		regulator_disable(priv->vdd_io);
+	regulator_disable(priv->vdd_reg);
+	regulator_disable(priv->vdd_io);
 	free_netdev(priv->netdev);
 
 	return 0;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] net: ks8851: Don't use regulator_get_optional()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

On Wed, May 28, 2014 at 01:11:12PM -0700, Stephen Boyd wrote:
> We shouldn't be using regulator_get_optional() here. These
> regulators are always present as part of the physical design and
> there isn't any way to use an internal regulator or change the
> source of the reference voltage via software. Given that the only
> users of this driver in the kernel are DT based, this change
> should be transparent to them even if they don't specify any
> supplies because the regulator framework will insert dummy
> supplies as needed.

Reviewed-by: Mark Brown <broonie@linaro.org>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] net: ks8851: Don't use regulator_get_optional()

[Nishanth Menon]

On Wed, May 28, 2014 at 3:11 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> We shouldn't be using regulator_get_optional() here. These
> regulators are always present as part of the physical design and
> there isn't any way to use an internal regulator or change the
> source of the reference voltage via software. Given that the only
> users of this driver in the kernel are DT based, this change
> should be transparent to them even if they don't specify any
> supplies because the regulator framework will insert dummy
> supplies as needed.
>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---

Reviewed-and-Tested-by: Nishanth Menon <nm@ti.com>

SDP2430: with next-20140529:
before patch: https://github.com/nmenon/kernel-test-logs/blob/next-20140529/omap2plus_defconfig/sdp2430.txt

After patch: http://slexy.org/raw/s21sryFhAx
Looks good.
---
Regards,
Nishanth Menon

Re: [PATCH] net: ks8851: Don't use regulator_get_optional()

[David Miller]

From: Stephen Boyd <sboyd@codeaurora.org>
Date: Wed, 28 May 2014 13:11:12 -0700

> We shouldn't be using regulator_get_optional() here. These
> regulators are always present as part of the physical design and
> there isn't any way to use an internal regulator or change the
> source of the reference voltage via software. Given that the only
> users of this driver in the kernel are DT based, this change
> should be transparent to them even if they don't specify any
> supplies because the regulator framework will insert dummy
> supplies as needed.
> 
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

This patch only applies to net-next, please make that explicit in your
future submissions by saying something like "[PATCH net-next] ..."
in your Subject line.

Applied, thanks.