Skip to contentSkip to conversation

net: ks8851: Don't use regulator_get_optional()

PATCH Applied Stephen Boyd 4 participants 4 messages May 28, 2014 – Jun 2, 2014

Subsystems (3)
Networking Network drivers Arm Qualcomm mailing list
lore
Share & tools
lore

Stephen Boyd

author
#0 lore raw

PATCH 1/1 net: ks8851: Don't use regulator_get_optional()

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 <[email protected]>
Cc Mark Brown <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>
---
 drivers/net/ethernet/micrel/ks8851.c | 50 ++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 28 deletions(-)
drivers/net/ethernet/micrel/ks8851.c +22 -28
index e72918970a58..66d4ab703f45 100644
@@ -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;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
Reviewed-by on v1 1/1
#1 lore raw
Quoted 8 lines · 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.

Reviewed-by Mark Brown <[email protected]>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140528/1c7cff8e/attachment.sig>

#2 lore raw
In reply to Stephen Boyd
Quoted 9 lines · 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.

Quoted 4 lines
Cc Nishanth Menon <[email protected]>
Cc Mark Brown <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>

---

Reviewed-and-Tested-by Nishanth Menon <[email protected]>

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

maintainer Applied
#3 lore raw
In reply to Stephen Boyd

From: Stephen Boyd <[email protected]>
Date: Wed, 28 May 2014 13:11:12 -0700

Quoted 9 lines

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.

Quoted 3 lines
Cc Nishanth Menon <[email protected]>
Cc Mark Brown <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>

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.