> -----Original Message----- > From: Todd Poynor [mailto:toddpoynor@xxxxxxxxxx] > Sent: Tuesday, July 12, 2011 4:41 AM > To: DebBarma, Tarun Kanti > Cc: linux-omap@xxxxxxxxxxxxxxx; Hilman, Kevin; Shilimkar, Santosh; > tony@xxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Gopinath, Thara > Subject: Re: [PATCH v14 05/12] OMAP: dmtimer: platform driver > > On Mon, Jul 11, 2011 at 04:59:12PM +0530, Tarun Kanti DebBarma wrote: > > Add dmtimer platform driver functions which include: > > (1) platform driver initialization > > (2) driver probe function > > (3) driver remove function > > > ... > > + timer = kzalloc(sizeof(struct omap_dm_timer), GFP_KERNEL); > > + if (!timer) { > > + dev_err(&pdev->dev, "%s: no memory for omap_dm_timer.\n", > > + __func__); > > + ret = -ENOMEM; > > + goto err_release_ioregion; > > + } > > + > > + timer->io_base = ioremap(mem->start, resource_size(mem)); > > + if (!timer->io_base) { > > + dev_err(&pdev->dev, "%s: ioremap failed.\n", __func__); > > + ret = -ENOMEM; > > + goto err_free_mem; > > + } > > + > > + if (pdata->timer_ip_type == OMAP_TIMER_IP_VERSION_2) { > > + timer->func_offset = VERSION2_TIMER_WAKEUP_EN_REG_OFFSET; > > + timer->intr_offset = VERSION2_TIMER_STAT_REG_OFFSET; > > + } else { > > + timer->func_offset = 0; > > + timer->intr_offset = 0; > > Don't need to init kzalloc'ed fields to zero. Yes, I will remove. > > > + } > > + > > + timer->id = pdev->id; > > + timer->irq = irq->start; > > + timer->pdev = pdev; > > + timer->reserved = 0; > > And here. Yes. > > > +err_free_pdev: > > + kfree(pdata); > > pdata wasn't allocated here, shouldn't be freed here. Yes, I will correct the error handling labels appropriately. > > > + platform_device_unregister(pdev); > > The platform device wasn't registered here. Oops... I will remove. Thanks. > > ... > > +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) { > > > Don't need list_for_each_entry_safe, you have the lock protecting list > modifications held and IRQs off. Ok. > > > + if (timer->pdev->id == pdev->id) { > > + kfree(timer->pdev->dev.platform_data); > > > Not allocated by this driver. In fact it gets automatically freed. I will remove. > > > + platform_device_del(timer->pdev); > > > Not added by this driver. Ok. -- Tarun > > > + list_del(&timer->node); > > + kfree(timer); > > > Todd -- 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
