Patchwork [1/2] mtd: gpmi-nand: Convert to platform driver

login
register
mail settings
Submitter Fabio Estevam
Date Sept. 5, 2012, 2:35 p.m.
Message ID <1346855725-31726-1-git-send-email-festevam@gmail.com>
Download mbox | patch
Permalink /patch/181880/
State Accepted
Commit 490e280a69cb0707ccb4c7e0c5c0be02d8ae102c
Headers show

Comments

Fabio Estevam - Sept. 5, 2012, 2:35 p.m.
From: Fabio Estevam <fabio.estevam@freescale.com>

Using module_platform_driver() makes the code smaller and cleaner.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
This patch depends on: "mtd: gpmi-nand: Improve logging style"

 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   29 +++++++----------------------
 1 file changed, 7 insertions(+), 22 deletions(-)
Marek Vasut - Sept. 5, 2012, 6:55 p.m.
Dear Fabio Estevam,

> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Using module_platform_driver() makes the code smaller and cleaner.

You did not document the dev_info() etc. addition, dunno if it's important or 
not.

Otherwise,

Reviewed-by: Marek Vasut <marex@denx.de>

> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> This patch depends on: "mtd: gpmi-nand: Improve logging style"
> 
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   29
> +++++++---------------------- 1 file changed, 7 insertions(+), 22
> deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index 9493507..5999b15 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1651,6 +1651,8 @@ static int __devinit gpmi_nand_probe(struct
> platform_device *pdev) if (ret)
>  		goto exit_nfc_init;
> 
> +	dev_info(this->dev, "driver registered.\n");
> +
>  	return 0;
> 
>  exit_nfc_init:
> @@ -1658,10 +1660,12 @@ exit_nfc_init:
>  exit_acquire_resources:
>  	platform_set_drvdata(pdev, NULL);
>  	kfree(this);
> +	dev_err(this->dev, "driver registration failed: %d\n", ret);
> +
>  	return ret;
>  }
> 
> -static int __exit gpmi_nand_remove(struct platform_device *pdev)
> +static int __devexit gpmi_nand_remove(struct platform_device *pdev)
>  {
>  	struct gpmi_nand_data *this = platform_get_drvdata(pdev);
> 
> @@ -1678,29 +1682,10 @@ static struct platform_driver gpmi_nand_driver = {
>  		.of_match_table = gpmi_nand_id_table,
>  	},
>  	.probe   = gpmi_nand_probe,
> -	.remove  = __exit_p(gpmi_nand_remove),
> +	.remove  = __devexit_p(gpmi_nand_remove),
>  	.id_table = gpmi_ids,
>  };
> -
> -static int __init gpmi_nand_init(void)
> -{
> -	int err;
> -
> -	err = platform_driver_register(&gpmi_nand_driver);
> -	if (err == 0)
> -		pr_info("driver registered.\n");
> -	else
> -		pr_err("driver registration failed.\n");
> -	return err;
> -}
> -
> -static void __exit gpmi_nand_exit(void)
> -{
> -	platform_driver_unregister(&gpmi_nand_driver);
> -}
> -
> -module_init(gpmi_nand_init);
> -module_exit(gpmi_nand_exit);
> +module_platform_driver(gpmi_nand_driver);
> 
>  MODULE_AUTHOR("Freescale Semiconductor, Inc.");
>  MODULE_DESCRIPTION("i.MX GPMI NAND Flash Controller Driver");

Best regards,
Marek Vasut
Fabio Estevam - Sept. 5, 2012, 7 p.m.
Hi Marek,

On Wed, Sep 5, 2012 at 3:55 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Fabio Estevam,
>
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Using module_platform_driver() makes the code smaller and cleaner.
>
> You did not document the dev_info() etc. addition, dunno if it's important or
> not.

It is not an addition. I just moved  "pr_info("driver registered.\n")"
 from gpmi_nand_init, (which is removed by this patch) to the probe
function.

Regards,

Fabio Estevam
Marek Vasut - Sept. 5, 2012, 7:23 p.m.
Dear Fabio Estevam,

> Hi Marek,
> 
> On Wed, Sep 5, 2012 at 3:55 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Fabio Estevam,
> > 
> >> From: Fabio Estevam <fabio.estevam@freescale.com>
> >> 
> >> Using module_platform_driver() makes the code smaller and cleaner.
> > 
> > You did not document the dev_info() etc. addition, dunno if it's
> > important or not.
> 
> It is not an addition. I just moved  "pr_info("driver registered.\n")"
>  from gpmi_nand_init, (which is removed by this patch) to the probe
> function.

Ah, just noticed. I'd say drop it completely then, there's now value in this 
print (or make it pr_debug() ). Besides, the init call was called only once if 
compiled in / every time if built as a module. The probe call is done per GPMI 
nand driver instance.

> Regards,
> 
> Fabio Estevam

Best regards,
Marek Vasut
Fabio Estevam - Sept. 5, 2012, 7:51 p.m.
On Wed, Sep 5, 2012 at 4:23 PM, Marek Vasut <marex@denx.de> wrote:

> Ah, just noticed. I'd say drop it completely then, there's now value in this
> print (or make it pr_debug() ). Besides, the init call was called only once if

I think we can keep this info. It does not hurt and this is how the
current driver does.

> compiled in / every time if built as a module. The probe call is done per GPMI
> nand driver instance.

,which is fine too.

Regards,

Fabio Estevam
Fabio Estevam - Sept. 21, 2012, 12:19 p.m.
Hi Artem.

On Wed, Sep 5, 2012 at 11:35 AM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Using module_platform_driver() makes the code smaller and cleaner.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> This patch depends on: "mtd: gpmi-nand: Improve logging style"

Any comments, please?

Regards,

Fabio Estevam
Marek Vasut - Sept. 21, 2012, 12:53 p.m.
Dear Fabio Estevam,

> Hi Artem.
> 
> On Wed, Sep 5, 2012 at 11:35 AM, Fabio Estevam <festevam@gmail.com> wrote:
> > From: Fabio Estevam <fabio.estevam@freescale.com>
> > 
> > Using module_platform_driver() makes the code smaller and cleaner.
> > 
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > ---
> > This patch depends on: "mtd: gpmi-nand: Improve logging style"
> 
> Any comments, please?

WFM

Reviewed-by: Marek Vasut <marex@denx.de>

> Regards,
> 
> Fabio Estevam

Best regards,
Marek Vasut
Artem Bityutskiy - Sept. 22, 2012, 2:59 p.m.
On Wed, 2012-09-05 at 11:35 -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Using module_platform_driver() makes the code smaller and cleaner.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Pushed both to l2-mtd.git, thanks!

Patch

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 9493507..5999b15 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1651,6 +1651,8 @@  static int __devinit gpmi_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_nfc_init;
 
+	dev_info(this->dev, "driver registered.\n");
+
 	return 0;
 
 exit_nfc_init:
@@ -1658,10 +1660,12 @@  exit_nfc_init:
 exit_acquire_resources:
 	platform_set_drvdata(pdev, NULL);
 	kfree(this);
+	dev_err(this->dev, "driver registration failed: %d\n", ret);
+
 	return ret;
 }
 
-static int __exit gpmi_nand_remove(struct platform_device *pdev)
+static int __devexit gpmi_nand_remove(struct platform_device *pdev)
 {
 	struct gpmi_nand_data *this = platform_get_drvdata(pdev);
 
@@ -1678,29 +1682,10 @@  static struct platform_driver gpmi_nand_driver = {
 		.of_match_table = gpmi_nand_id_table,
 	},
 	.probe   = gpmi_nand_probe,
-	.remove  = __exit_p(gpmi_nand_remove),
+	.remove  = __devexit_p(gpmi_nand_remove),
 	.id_table = gpmi_ids,
 };
-
-static int __init gpmi_nand_init(void)
-{
-	int err;
-
-	err = platform_driver_register(&gpmi_nand_driver);
-	if (err == 0)
-		pr_info("driver registered.\n");
-	else
-		pr_err("driver registration failed.\n");
-	return err;
-}
-
-static void __exit gpmi_nand_exit(void)
-{
-	platform_driver_unregister(&gpmi_nand_driver);
-}
-
-module_init(gpmi_nand_init);
-module_exit(gpmi_nand_exit);
+module_platform_driver(gpmi_nand_driver);
 
 MODULE_AUTHOR("Freescale Semiconductor, Inc.");
 MODULE_DESCRIPTION("i.MX GPMI NAND Flash Controller Driver");