diff mbox

[v4,2/4] mtd: nand: gpio: Use devm_gpio_request_one() where possible

Message ID 1376637633-12540-1-git-send-email-shc_work@mail.ru
State New, archived
Headers show

Commit Message

Alexander Shiyan Aug. 16, 2013, 7:20 a.m. UTC 
Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/mtd/nand/gpio.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

Comments

Fabio Estevam Aug. 16, 2013, 11:06 a.m. UTC | #1 
On Fri, Aug 16, 2013 at 4:20 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>

You are not doing only what you describe in the Subject line.

It would be nice to provide a commit log.
Alexander Shiyan Aug. 16, 2013, 12:11 p.m. UTC | #2 
> On Fri, Aug 16, 2013 at 4:20 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> >
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> 
> You are not doing only what you describe in the Subject line.
> 
> It would be nice to provide a commit log.

Is it worth it to make the next version of the patch?
On my opinion, everything is obvious from the name.

---
Brian Norris Aug. 17, 2013, 8:31 p.m. UTC | #3 
On Fri, Aug 16, 2013 at 04:11:01PM +0400, Alexander Shiyan wrote:
> > On Fri, Aug 16, 2013 at 4:20 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> > >
> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > 
> > You are not doing only what you describe in the Subject line.
> > 
> > It would be nice to provide a commit log.
> 
> Is it worth it to make the next version of the patch?
> On my opinion, everything is obvious from the name.

Half of it is obvious, but the part in which you add gpio_nand_set_wp()
and use it is not related to the subject. It's really a separate bit of
refactoring, it seems, which could be valid only if you mention it.
Otherwise, a subject-line-only commit message means the patch must do
one thing only.

You can just resend this one patch, or even split it into two patches. I
believe the rest should be OK. (I'm looking now.)

Thanks,
Brian
diff mbox

Patch

diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index 8eea181..98cc714 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -190,14 +190,20 @@  gpio_nand_get_io_sync(struct platform_device *pdev)
 	return platform_get_resource(pdev, IORESOURCE_MEM, 1);
 }
 
+static void gpio_nand_set_wp(struct gpiomtd *gpiomtd, int val)
+{
+	if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
+		gpio_set_value(gpiomtd->plat.gpio_nwp, val);
+}
+
 static int gpio_nand_remove(struct platform_device *pdev)
 {
 	struct gpiomtd *gpiomtd = platform_get_drvdata(pdev);
 
 	nand_release(&gpiomtd->mtd_info);
 
-	if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
-		gpio_set_value(gpiomtd->plat.gpio_nwp, 0);
+	gpio_nand_set_wp(gpiomtd, 0);
+
 	gpio_set_value(gpiomtd->plat.gpio_nce, 1);
 
 	return 0;
@@ -242,34 +248,33 @@  static int gpio_nand_probe(struct platform_device *pdev)
 			return PTR_ERR(gpiomtd->io_sync);
 	}
 
-	ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_nce, "NAND NCE");
+	ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_nce,
+				    GPIOF_OUT_INIT_HIGH, "NAND NCE");
 	if (ret)
 		return ret;
-	gpio_direction_output(gpiomtd->plat.gpio_nce, 1);
 
 	if (gpio_is_valid(gpiomtd->plat.gpio_nwp)) {
-		ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_nwp,
-					"NAND NWP");
+		ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_nwp,
+					    GPIOF_OUT_INIT_LOW, "NAND NWP");
 		if (ret)
 			return ret;
 	}
 
-	ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_ale, "NAND ALE");
+	ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_ale,
+				    GPIOF_OUT_INIT_LOW, "NAND ALE");
 	if (ret)
 		return ret;
-	gpio_direction_output(gpiomtd->plat.gpio_ale, 0);
 
-	ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_cle, "NAND CLE");
+	ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_cle,
+				    GPIOF_OUT_INIT_LOW, "NAND CLE");
 	if (ret)
 		return ret;
-	gpio_direction_output(gpiomtd->plat.gpio_cle, 0);
 
 	if (gpio_is_valid(gpiomtd->plat.gpio_rdy)) {
-		ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_rdy,
-					"NAND RDY");
+		ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_rdy,
+					    GPIOF_IN, "NAND RDY");
 		if (ret)
 			return ret;
-		gpio_direction_input(gpiomtd->plat.gpio_rdy);
 		chip->dev_ready = gpio_nand_devready;
 	}
 
@@ -284,8 +289,7 @@  static int gpio_nand_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, gpiomtd);
 
-	if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
-		gpio_direction_output(gpiomtd->plat.gpio_nwp, 1);
+	gpio_nand_set_wp(gpiomtd, 1);
 
 	if (nand_scan(&gpiomtd->mtd_info, 1)) {
 		ret = -ENXIO;
@@ -304,8 +308,7 @@  static int gpio_nand_probe(struct platform_device *pdev)
 		return 0;
 
 err_wp:
-	if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
-		gpio_set_value(gpiomtd->plat.gpio_nwp, 0);
+	gpio_nand_set_wp(gpiomtd, 0);
 
 	return ret;
 }