> -----Original Message-----
> From: G, Manjunath Kondaiah
> Sent: Tuesday, December 07, 2010 11:18 AM
> To: DebBarma, Tarun Kanti
> Cc: linux-omap@xxxxxxxxxxxxxxx; Gopinath, Thara
> Subject: Re: [PATCH v5 9/12] OMAP: dmtimer: platform driver
>
> * DebBarma, Tarun Kanti <tarun.kanti@xxxxxx> [2010-12-07 11:02:26 +0530]:
>
> > > -----Original Message-----
> > > From: G, Manjunath Kondaiah
> > > Sent: Tuesday, December 07, 2010 12:40 AM
> > > To: DebBarma, Tarun Kanti
> > > Cc: linux-omap@xxxxxxxxxxxxxxx; Gopinath, Thara
> > > Subject: Re: [PATCH v5 9/12] OMAP: dmtimer: platform driver
> > >
> > > On Tue, Dec 07, 2010 at 05:14:16AM +0530, Tarun Kanti DebBarma wrote:
> > > > From: Thara Gopinath <thara@xxxxxx>
> > > >
> > > > Add dmtimer platform driver functions which include:
> > > > (1) platform driver initialization
> > > > (2) driver probe function
> > > > (3) driver remove function
> > > >
> > > > Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@xxxxxx>
> > > > Signed-off-by: Thara Gopinath <thara@xxxxxx>
> > > > Reviewed-by: Cousson, Benoit <b-cousson@xxxxxx>
>
> [...]
>
> > > > +
> > > > +err_free_mem:
> > > > + kfree(timer);
> > > > +
> > > > +err_release_ioregion:
> > > > + release_mem_region(mem->start, resource_size(mem));
> > > > +
> > > > +err_free_pdev:
> > > You can also free pdata?
> > This pdata points to data within pdev created by omap_device_build.
> > But the pdata which was allocated locally is already freed
> > After omap_device_build() in omap_timer_init() in mach-omap.
>
> You should also free memory allocated for pdata during omap_device_build
> since you no longer require pdata due to probe fail.
I am not denying this. So, your comment you have been more easily understood
If you have mentioned that platform_device_del does not free pdata.
Therefore, it must be explicitly freed.
>
> >
> > > > + platform_device_del(pdev);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +/**
> > > > + * omap_dm_timer_remove - cleanup a registered timer device
> > > > + * @pdev: pointer to current timer platform device
> > > > + *
> > > > + * Called by driver framework whenever a timer device is
> unregistered.
> > > > + * In addition to freeing platform resources it also deletes the
> timer
> > > > + * entry from the local list.
> > > > + */
> > > > +static int __devexit omap_dm_timer_remove(struct platform_device
> *pdev)
> > > > +{
> > > > + struct omap_dm_timer *timer, *tmp;
> > > > + unsigned long flags;
> > > > + int ret = -EINVAL;
> > > > +
> > > > + spin_lock_irqsave(&dm_timer_lock, flags);
> > > > + list_for_each_entry_safe(timer, tmp, &omap_timer_list, node) {
> > > > + if (timer->pdev->id == pdev->id) {
> > > > + platform_device_del(timer->pdev);
> > > > + list_del(&timer->node);
> > > > + kfree(timer);
> > > kfree(pdev->dev.platform_data);
> > Ok, this is supposed to be done as part of platform_device_del above.
>
> No. platform_device_del will not free platform_data. You have use kfree
> for
> freeing pdata or platform_data.
I will confirm and change accordingly.
--
Tarun
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
