Re: [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup

[Felipe Contreras <felipe.contreras@nokia.com>]

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