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

Felipe Contreras
2010-02-18T14:27:42Z

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