Messages in this thread | | | Date | Tue, 26 Aug 2014 14:19:18 +0300 | From | Tomi Valkeinen <> | Subject | Re: [PATCH] msm: msm_fb: Add remove function for platform driver for clean unloading |
| |
Hi,
On 04/08/14 17:43, Pramod Gurav wrote: > This adds a remove function to platform driver structure so that > resources are released when driver is unloaded. > > Moved kzalloc to use managed resource and adds a return check if failed.
Please split this patch into two. You change two unrelated things here, the unloading and the kzalloc part.
> CC: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> > CC: Tomi Valkeinen <tomi.valkeinen@ti.com> > CC: Stephen Boyd <sboyd@codeaurora.org> > CC: Jingoo Han <jg1.han@samsung.com> > CC: Rob Clark <robdclark@gmail.com> > > Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com> > --- > drivers/video/fbdev/msm/msm_fb.c | 28 +++++++++++++++++++++++++--- > 1 file changed, 25 insertions(+), 3 deletions(-) > > diff --git a/drivers/video/fbdev/msm/msm_fb.c b/drivers/video/fbdev/msm/msm_fb.c > index 1374803..ac277e2 100644 > --- a/drivers/video/fbdev/msm/msm_fb.c > +++ b/drivers/video/fbdev/msm/msm_fb.c > @@ -553,6 +553,7 @@ static int msmfb_probe(struct platform_device *pdev) > fb = framebuffer_alloc(sizeof(struct msmfb_info), &pdev->dev); > if (!fb) > return -ENOMEM; > + > msmfb = fb->par; > msmfb->fb = fb; > msmfb->panel = panel; > @@ -569,8 +570,13 @@ static int msmfb_probe(struct platform_device *pdev) > mutex_init(&msmfb->panel_init_lock); > init_waitqueue_head(&msmfb->frame_wq); > INIT_WORK(&msmfb->resume_work, power_on_panel); > - msmfb->black = kzalloc(msmfb->fb->var.bits_per_pixel*msmfb->xres, > - GFP_KERNEL); > + msmfb->black = devm_kzalloc(&pdev->dev, > + msmfb->fb->var.bits_per_pixel*msmfb->xres, > + GFP_KERNEL); > + if (!msmfb->black) { > + ret = -ENOMEM; > + goto error_register_framebuffer; > + } > > printk(KERN_INFO "msmfb_probe() installing %d x %d panel\n", > msmfb->xres, msmfb->yres); > @@ -589,6 +595,8 @@ static int msmfb_probe(struct platform_device *pdev) > > msmfb->sleeping = WAKING; > > + platform_set_drvdata(pdev, msmfb); > + > return 0; > > error_register_framebuffer: > @@ -598,13 +606,27 @@ error_setup_fbmem: > return ret; > } > > +static int msmfb_remove(struct platform_device *pdev) > +{ > + struct msmfb_info *msmfb = NULL;
No need to initialize to NULL, msmfb is initialized always below.
> + > + msmfb = platform_get_drvdata(pdev); > + if (msmfb) {
Can msmfb ever be NULL here? If I'm not mistaken, remove is only called if the probe had succeeded. And if probe had succeeded, the drvdata is always set to a proper pointer to msmfb_info.
Tomi
[unhandled content-type:application/pgp-signature] | |