Hi Fernando,
On Thu, 2010-02-18 at 02:52 +0100, ext Guzman Lugo, Fernando wrote:
> Hi,
>
> >-----Original Message-----
> >From: Ameya Palande [mailto:[email protected]]
> >Sent: Wednesday, February 17, 2010 12:06 PM
> >To: [email protected]
> >Cc: [email protected]; Menon, Nishanth; Chitriki Rudramuni,
> >Deepak; Guzman Lugo, Fernando; Ramirez Luna, Omar
> >Subject: [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup
> >
> >This patch improves current mapped memory cleanup mechanism by using linux
> >native list implementation. As a side effect we also get following
> >benefits:
> >
> >1. Unnecessary data members in DMM_MAP_OBJECT are removed which results in
> > memory saving.
> >
> >2. Following functions are removed as they are not needed anymore:
> > DRV_ProcFreeDMMRes()
> > DRV_UpdateDMMResElement()
> > DRV_InsertDMMResElement()
> > DRV_GetDMMResElement()
> > DRV_RemoveDMMResElement()
> >
> >Signed-off-by: Ameya Palande <[email protected]>
> >---
> >@@ -1334,11 +1333,17 @@ DSP_STATUS PROC_Map(DSP_HPROCESSOR hProcessor, void
> >*pMpuAddr, u32 ulSize,
> > }
> > (void)SYNC_LeaveCS(hProcLock);
> >
> >- if (DSP_SUCCEEDED(status)) {
> >- DRV_InsertDMMResElement(&dmmRes, pr_ctxt);
> >- DRV_UpdateDMMResElement(dmmRes, (u32)pMpuAddr, ulSize,
> >- (u32)pReqAddr, (u32)*ppMapAddr, hProcessor);
> >+ if (DSP_FAILED(status))
> >+ goto func_end;
> >+
> >+ 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);
> > }
>
> 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.
> 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 ;)
Cheers,
Ameya.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
