| Message ID | 001601cefd40$f6b3dd00$e41b9700$%han@samsung.com |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 20, 2013 at 02:04:29PM +0900, Jingoo Han wrote: > --- a/drivers/mtd/nand/plat_nand.c > +++ b/drivers/mtd/nand/plat_nand.c > @@ -52,25 +52,16 @@ static int plat_nand_probe(struct platform_device *pdev) > return -ENXIO; > > /* Allocate memory for the device structure (and zero it) */ > - data = kzalloc(sizeof(struct plat_nand_data), GFP_KERNEL); > + data = devm_kzalloc(&pdev->dev, sizeof(struct plat_nand_data), > + GFP_KERNEL); > if (!data) { > dev_err(&pdev->dev, "failed to allocate device structure.\n"); > return -ENOMEM; > } > > - if (!request_mem_region(res->start, resource_size(res), > - dev_name(&pdev->dev))) { > - dev_err(&pdev->dev, "request_mem_region failed\n"); > - err = -EBUSY; > - goto out_free; > - } > - > - data->io_base = ioremap(res->start, resource_size(res)); > - if (data->io_base == NULL) { > - dev_err(&pdev->dev, "ioremap failed\n"); > - err = -EIO; > - goto out_release_io; > - } > + data->io_base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(data->io_base)) > + return PTR_ERR(data->io_base); This gives me a few errors: drivers/mtd/nand/plat_nand.c: In function 'plat_nand_probe': drivers/mtd/nand/plat_nand.c:61:2: error: implicit declaration of function 'IS_ERR' [-Werror=implicit-function-declaration] drivers/mtd/nand/plat_nand.c:62:3: error: implicit declaration of function 'PTR_ERR' [-Werror=implicit-function-declaration] make[4]: *** [drivers/mtd/nand/plat_nand.o] Error 1 You probably need to #include <linux/err.h>. I guess you don't compile-test your changes? It would help if you can catch these mistakes before they get to the maintainer. > > data->chip.priv = &data; > data->mtd.priv = &data->chip; Brian
On Thu, Jan 2, 2014 at 5:17 PM, Brian Norris <computersforpeace@gmail.com> wrote: > On Fri, Dec 20, 2013 at 02:04:29PM +0900, Jingoo Han wrote: >> + if (IS_ERR(data->io_base)) >> + return PTR_ERR(data->io_base); > > This gives me a few errors: > > drivers/mtd/nand/plat_nand.c: In function 'plat_nand_probe': > drivers/mtd/nand/plat_nand.c:61:2: error: implicit declaration of function 'IS_ERR' [-Werror=implicit-function-declaration] > drivers/mtd/nand/plat_nand.c:62:3: error: implicit declaration of function 'PTR_ERR' [-Werror=implicit-function-declaration] > make[4]: *** [drivers/mtd/nand/plat_nand.o] Error 1 > > You probably need to #include <linux/err.h>. I guess you don't > compile-test your changes? It would help if you can catch these mistakes > before they get to the maintainer. > >> >> data->chip.priv = &data; >> data->mtd.priv = &data->chip; Sorry, I replied to the v1, but I was actually testing v3. My comments apply to either version though. Brian
On Friday, January 03, 2014 10:18 AM, Jingoo Han wrote: > On Fri, Dec 20, 2013 at 02:04:29PM +0900, Jingoo Han wrote: > > --- a/drivers/mtd/nand/plat_nand.c > > +++ b/drivers/mtd/nand/plat_nand.c > > @@ -52,25 +52,16 @@ static int plat_nand_probe(struct platform_device *pdev) > > return -ENXIO; > > > > /* Allocate memory for the device structure (and zero it) */ > > - data = kzalloc(sizeof(struct plat_nand_data), GFP_KERNEL); > > + data = devm_kzalloc(&pdev->dev, sizeof(struct plat_nand_data), > > + GFP_KERNEL); > > if (!data) { > > dev_err(&pdev->dev, "failed to allocate device structure.\n"); > > return -ENOMEM; > > } > > > > - if (!request_mem_region(res->start, resource_size(res), > > - dev_name(&pdev->dev))) { > > - dev_err(&pdev->dev, "request_mem_region failed\n"); > > - err = -EBUSY; > > - goto out_free; > > - } > > - > > - data->io_base = ioremap(res->start, resource_size(res)); > > - if (data->io_base == NULL) { > > - dev_err(&pdev->dev, "ioremap failed\n"); > > - err = -EIO; > > - goto out_release_io; > > - } > > + data->io_base = devm_ioremap_resource(&pdev->dev, res); > > + if (IS_ERR(data->io_base)) > > + return PTR_ERR(data->io_base); > > This gives me a few errors: > > drivers/mtd/nand/plat_nand.c: In function 'plat_nand_probe': > drivers/mtd/nand/plat_nand.c:61:2: error: implicit declaration of function 'IS_ERR' [-Werror=implicit- > function-declaration] > drivers/mtd/nand/plat_nand.c:62:3: error: implicit declaration of function 'PTR_ERR' [- > Werror=implicit-function-declaration] > make[4]: *** [drivers/mtd/nand/plat_nand.o] Error 1 > > You probably need to #include <linux/err.h>. I guess you don't > compile-test your changes? It would help if you can catch these mistakes > before they get to the maintainer. Hi Norris, I did compile all patches. However, I cannot understand why the build error happens. Anyway, I will check it and add '#include <linux/err.h' if needed. Thank you for your comment. :-) Happy new year! Best regards, Jingoo Han
On Fri, Jan 03, 2014 at 10:27:53AM +0900, Jingoo Han wrote: > On Friday, January 03, 2014 10:18 AM, Jingoo Han wrote: ^^^ Huh? It looks like you quoted me, but your mailer says you're quoting yourself! You might want to fix that :) > > This gives me a few errors: > > > > drivers/mtd/nand/plat_nand.c: In function 'plat_nand_probe': > > drivers/mtd/nand/plat_nand.c:61:2: error: implicit declaration of function 'IS_ERR' [-Werror=implicit- > > function-declaration] > > drivers/mtd/nand/plat_nand.c:62:3: error: implicit declaration of function 'PTR_ERR' [- > > Werror=implicit-function-declaration] > > make[4]: *** [drivers/mtd/nand/plat_nand.o] Error 1 > > > > You probably need to #include <linux/err.h>. I guess you don't > > compile-test your changes? It would help if you can catch these mistakes > > before they get to the maintainer. > > Hi Norris, > > I did compile all patches. However, I cannot understand why the build > error happens. I guess you were relying on an implicit #include via some other explicitly-included header, and maybe this implicit inclusion is different for different ARCHes, so that this only fails to compile under certain configurations. > Anyway, I will check it and add '#include <linux/err.h' > if needed. Yes, that is the right thing. Standard practice is to explicitly include any required headers, so that changes in unrelated headers don't break things. > Thank you for your comment. :-) > Happy new year! Yes, happy 2014! Brian
diff --git a/drivers/mtd/nand/plat_nand.c b/drivers/mtd/nand/plat_nand.c index cad4cdc..7734f0e 100644 --- a/drivers/mtd/nand/plat_nand.c +++ b/drivers/mtd/nand/plat_nand.c @@ -52,25 +52,16 @@ static int plat_nand_probe(struct platform_device *pdev) return -ENXIO; /* Allocate memory for the device structure (and zero it) */ - data = kzalloc(sizeof(struct plat_nand_data), GFP_KERNEL); + data = devm_kzalloc(&pdev->dev, sizeof(struct plat_nand_data), + GFP_KERNEL); if (!data) { dev_err(&pdev->dev, "failed to allocate device structure.\n"); return -ENOMEM; } - if (!request_mem_region(res->start, resource_size(res), - dev_name(&pdev->dev))) { - dev_err(&pdev->dev, "request_mem_region failed\n"); - err = -EBUSY; - goto out_free; - } - - data->io_base = ioremap(res->start, resource_size(res)); - if (data->io_base == NULL) { - dev_err(&pdev->dev, "ioremap failed\n"); - err = -EIO; - goto out_release_io; - } + data->io_base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(data->io_base)) + return PTR_ERR(data->io_base); data->chip.priv = &data; data->mtd.priv = &data->chip; @@ -122,11 +113,6 @@ static int plat_nand_probe(struct platform_device *pdev) out: if (pdata->ctrl.remove) pdata->ctrl.remove(pdev); - iounmap(data->io_base); -out_release_io: - release_mem_region(res->start, resource_size(res)); -out_free: - kfree(data); return err; } @@ -137,16 +123,10 @@ static int plat_nand_remove(struct platform_device *pdev) { struct plat_nand_data *data = platform_get_drvdata(pdev); struct platform_nand_data *pdata = dev_get_platdata(&pdev->dev); - struct resource *res; - - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); nand_release(&data->mtd); if (pdata->ctrl.remove) pdata->ctrl.remove(pdev); - iounmap(data->io_base); - release_mem_region(res->start, resource_size(res)); - kfree(data); return 0; }
Use devm_*() functions to make cleanup paths simpler. Signed-off-by: Jingoo Han <jg1.han@samsung.com> --- drivers/mtd/nand/plat_nand.c | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-)