On Thu, Feb 18, 2010 at 01:15:55PM +0100, Ameya Palande wrote:
> On Thu, 2010-02-18 at 02:52 +0100, ext Guzman Lugo, Fernando wrote:
> > What do you think about it?
> > Instead of removing DRV_InsertDMMResElement make it an inline function with your code inside:
>
> Making DRV_InsertDMMResElement inline doesn't make sense since we are
> not calling it multiple times.
I agree with Fernando; the code would be more readable. Even more if we
rename the function to instert_map_element().
note: should be 'static inline'
> > inline void DRV_InsertDMMResElement(u32 ppMapAddr)
> > {
> > map_obj = kmalloc(sizeof(struct DMM_MAP_OBJECT), GFP_KERNEL);
> > if (map_obj) {
> > map_obj->dsp_addr = (u32)*ppMapAddr;
> > spin_lock(&pr_ctxt->dmm_map_lock);
> > list_add(&map_obj->link, &pr_ctxt->dmm_map_list);
> > spin_unlock(&pr_ctxt->dmm_map_lock);
> > }
> > }
> >
> > It could make the code more understandable about what it is actually doing.
> > It also applies to the functions which removes this patch.
>
> I can put a comment here to clarify what this code is doing ;)
AFAIK in linux, self-documenting code is preferred over comments.
Cheers.
--
Felipe Contreras
--
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
