[PATCH 1/3] serial: amba-pl011: Fix regulator_get() return check

[PATCH 1/3] serial: amba-pl011: Fix regulator_get() return check

[Andrey Smirnov]

NULL is used to designate a dummy regulator, so it it should be safe
to use against regulator_enable(). Any value that would retrun true
for IS_ERR(), OTOH, is not. Such value would also pass "if (r)" check
without any problems.

Fix the code to use !IS_ERR() instead.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/serial/amba-pl011.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/serial/amba-pl011.c b/drivers/serial/amba-pl011.c
index 4c4067d5b..ce40f840f 100644
--- a/drivers/serial/amba-pl011.c
+++ b/drivers/serial/amba-pl011.c
@@ -185,7 +185,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 	struct regulator *r;
 
 	r = regulator_get(&dev->dev, NULL);
-	if (r) {
+	if (!IS_ERR(r)) {
 		int ret;
 
 		ret = regulator_enable(r);
-- 
2.19.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] mci: Rely on NULL being a dummy regulator

[Andrey Smirnov]

Since NULL, is a dummy regulator, we can drop a bit of error checking
logic and simplify the code if we assing host->supply to NULL in case
we can't find an appropriate regulator during probing.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/mci/mci-core.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 3efd80a8a..c71d91fd0 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1650,13 +1650,11 @@ static int mci_card_probe(struct mci *mci)
 		return -ENODEV;
 	}
 
-	if (!IS_ERR(host->supply)) {
-		ret = regulator_enable(host->supply);
-		if (ret) {
-			dev_err(&mci->dev, "failed to enable regulator: %s\n",
-				strerror(-ret));
-			return ret;
-		}
+	ret = regulator_enable(host->supply);
+	if (ret) {
+		dev_err(&mci->dev, "failed to enable regulator: %s\n",
+			strerror(-ret));
+		return ret;
 	}
 
 	/* start with a host interface reset */
@@ -1728,8 +1726,7 @@ on_error:
 	if (rc != 0) {
 		host->clock = 0;	/* disable the MCI clock */
 		mci_set_ios(mci);
-		if (!IS_ERR(host->supply))
-			regulator_disable(host->supply);
+		regulator_disable(host->supply);
 	}
 
 	return rc;
@@ -1816,8 +1813,10 @@ int mci_register(struct mci_host *host)
 	mci->dev.detect = mci_detect;
 
 	host->supply = regulator_get(host->hw_dev, "vmmc");
-	if (IS_ERR(host->supply))
+	if (IS_ERR(host->supply)) {
 		dev_err(&mci->dev, "Failed to get 'vmmc' regulator.\n");
+		host->supply = NULL;
+	}
 
 	ret = register_device(&mci->dev);
 	if (ret)
-- 
2.19.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] video: mtl017: Make sure errno is not used as a regulator pointer

[Andrey Smirnov]

Since regulator_get() can return an errno via regulator pointer, we
need to make sure we handle that case without passing bogus pointers
around.

Add code to convert mtl017->regulator to a dummy regulator if
regulator_get() fails.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/video/mtl017.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/mtl017.c b/drivers/video/mtl017.c
index 085ea110b..423fb8e4f 100644
--- a/drivers/video/mtl017.c
+++ b/drivers/video/mtl017.c
@@ -244,6 +244,9 @@ static int mtl017_probe(struct device_d *dev)
 	mtl017->client = to_i2c_client(dev);
 	mtl017->regulator = regulator_get(dev, "vdd");
 
+	if (IS_ERR(mtl017->regulator))
+		mtl017->regulator = NULL;
+
 	mtl017->enable_gpio = of_get_named_gpio_flags(dev->device_node,
 			"enable-gpios", 0, &flags);
 	if (gpio_is_valid(mtl017->enable_gpio)) {
-- 
2.19.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] serial: amba-pl011: Fix regulator_get() return check

[Sascha Hauer]

On Wed, Dec 12, 2018 at 11:10:31PM -0800, Andrey Smirnov wrote:
> NULL is used to designate a dummy regulator, so it it should be safe
> to use against regulator_enable(). Any value that would retrun true
> for IS_ERR(), OTOH, is not. Such value would also pass "if (r)" check
> without any problems.
> 
> Fix the code to use !IS_ERR() instead.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  drivers/serial/amba-pl011.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/serial/amba-pl011.c b/drivers/serial/amba-pl011.c
> index 4c4067d5b..ce40f840f 100644
> --- a/drivers/serial/amba-pl011.c
> +++ b/drivers/serial/amba-pl011.c
> @@ -185,7 +185,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>  	struct regulator *r;
>  
>  	r = regulator_get(&dev->dev, NULL);
> -	if (r) {
> +	if (!IS_ERR(r)) {
>  		int ret;
>  
>  		ret = regulator_enable(r);
> -- 
> 2.19.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox