linux-kernel - [RFC/RFT][PATCH 1/2] regulator: max8997: Properly handle gpio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <1341500817.14503.1.camel@phoenix>
Date:	Thu, 05 Jul 2012 23:06:57 +0800
From:	Axel Lin <axel.lin@...il.com>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Cc:	Kyungmin Park <kyungmin.park@...sung.com>,
	MyungJoo Ham <myungjoo.ham@...sung.com>,
	Liam Girdwood <lrg@...com>, linux-kernel@...r.kernel.org
Subject: [RFC/RFT][PATCH 1/2] regulator: max8997: Properly handle
 gpio_request failure

Convert to devm_gpio_request to save a few error handling code.

This patch properly handle the gpio_request failure with -EBUSY, we should
return error rather than ommit the gpio_request failure with -EBUSY.

I think one of the reason we got -EBUSY is because current code does not free
gpios in max8997_pmic_remove(). So it got -EBUSY when reload the module.

Yest another reason is in current code if gpio_request() returns -EBUSY,
the rest of the code still calls gpio_direction_output to config buck125_gpios
and set gpio value in max8997_set_gpio().  This looks wrong to me.

Signed-off-by: Axel Lin <axel.lin@...il.com>
---
 drivers/regulator/max8997.c |   40 +++++++++-------------------------------
 1 file changed, 9 insertions(+), 31 deletions(-)

diff --git a/drivers/regulator/max8997.c b/drivers/regulator/max8997.c
index 704cd49..e39a0c7 100644
--- a/drivers/regulator/max8997.c
+++ b/drivers/regulator/max8997.c
@@ -1025,7 +1025,6 @@ static __devinit int max8997_pmic_probe(struct platform_device *pdev)
 	 */
 	if (pdata->buck1_gpiodvs || pdata->buck2_gpiodvs ||
 			pdata->buck5_gpiodvs) {
-		bool gpio1set = false, gpio2set = false;
 
 		if (!gpio_is_valid(pdata->buck125_gpios[0]) ||
 				!gpio_is_valid(pdata->buck125_gpios[1]) ||
@@ -1035,40 +1034,20 @@ static __devinit int max8997_pmic_probe(struct platform_device *pdev)
 			goto err_out;
 		}
 
-		ret = gpio_request(pdata->buck125_gpios[0],
-				"MAX8997 SET1");
-		if (ret == -EBUSY)
-			dev_warn(&pdev->dev, "Duplicated gpio request"
-					" on SET1\n");
-		else if (ret)
+		ret = devm_gpio_request(&pdev->dev, pdata->buck125_gpios[0],
+					"MAX8997 SET1");
+		if (ret)
 			goto err_out;
-		else
-			gpio1set = true;
-
-		ret = gpio_request(pdata->buck125_gpios[1],
-				"MAX8997 SET2");
-		if (ret == -EBUSY)
-			dev_warn(&pdev->dev, "Duplicated gpio request"
-					" on SET2\n");
-		else if (ret) {
-			if (gpio1set)
-				gpio_free(pdata->buck125_gpios[0]);
+
+		ret = devm_gpio_request(&pdev->dev, pdata->buck125_gpios[1],
+					"MAX8997 SET2");
+		if (ret)
 			goto err_out;
-		} else
-			gpio2set = true;
 
-		ret = gpio_request(pdata->buck125_gpios[2],
+		ret = devm_gpio_request(&pdev->dev, pdata->buck125_gpios[2],
 				"MAX8997 SET3");
-		if (ret == -EBUSY)
-			dev_warn(&pdev->dev, "Duplicated gpio request"
-					" on SET3\n");
-		else if (ret) {
-			if (gpio1set)
-				gpio_free(pdata->buck125_gpios[0]);
-			if (gpio2set)
-				gpio_free(pdata->buck125_gpios[1]);
+		if (ret)
 			goto err_out;
-		}
 
 		gpio_direction_output(pdata->buck125_gpios[0],
 				(max8997->buck125_gpioindex >> 2)
@@ -1079,7 +1058,6 @@ static __devinit int max8997_pmic_probe(struct platform_device *pdev)
 		gpio_direction_output(pdata->buck125_gpios[2],
 				(max8997->buck125_gpioindex >> 0)
 				& 0x1); /* SET3 */
-		ret = 0;
 	}
 
 	/* DVS-GPIO disabled */
-- 
1.7.9.5



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/