| Message ID | 1346855725-31726-1-git-send-email-festevam@gmail.com |
|---|---|
| State | Accepted |
| Commit | 490e280a69cb0707ccb4c7e0c5c0be02d8ae102c |
| Headers | show |
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
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
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
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
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
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
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!
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");