Re: OMAP3 DSPBridge review

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Vijay,

On Thu, Apr 17, 2008 at 11:24 AM, Trilok Soni <soni.trilok@xxxxxxxxx> wrote:
> Hi Vijay,
>
>
>
>  On Thu, Apr 17, 2008 at 8:15 AM, Pasam, Vijay <vpasam@xxxxxx> wrote:
>  > Hi Trilok:
>  >
>  >
>  >
>  >  >
>  >  > I was just going this dspbridge code, and I was surprised to
>  >  > find layers for un-necessary things, where we could have
>  >  > simply called the linux kernel provided function. Here goes the list:
>  >  >
>  >  > 1. There is un-necessary "linux" directory inside mpu_driver
>  >  > source codes. e.g src/osal/linux etc. I hope we are writing
>  >  > this code for Linux kernel only.
>  >  > 2. src/osal/linux
>  >  >   - As the name suggest for directory it is OS Abstraction
>  >  > layer for Linux. I don't see any need for this at all. In
>  >  > Linux kernel, we follow approach of directly calling the
>  >  > provided functions and not to further wrap it until necessary.
>  >  >
>  >  > src/osal/list.c
>  >  >
>  >  > - This whole file tries implement doubly linked list code, I
>  >  > am sure you can get away this with list.h itself. I don't see
>  >  > any need of further wrapping it.
>  >  >
>  >  > src/osal/kfile.c
>  >  >
>  >  > - Don't use KENREL_VERSION comparision check unless very much
>  >  > necessary, always try to keep driver in sync in latest kernel
>  >  > version instead.
>  >  > - Again there are wrappers for file reading functions
>  >  > provided by kernel.
>  >  > - If this functions are only used for doff loader, then
>  >  > moving it to outside kernel space, then we can get away with
>  >  > this functions.
>  >  >
>  >  > src/osal/mem.c
>  >  >
>  >  > - I see here that you guys are maintaining the pool of memory
>  >  > allocated with dma_alloc_coherent, you guys should have used
>  >  > existing mempool apis instead, look at dspgw code for example.
>  >  >
>  >  > src/osal/isr.c
>  >  >
>  >  > - Not required, please call request_irq/free_irq directly
>  >  > please. I also see lots of #ifdef LINUX and #ifndef LINUX,
>  >  > does TI also target to build this code for WinCE and Symbian
>  >  > like OS. If yes, I don't see how we are going to maintain
>  >  > this along with community.
>  >  >
>  >  > src/osal/dpc.c - please call tasklet functions directly instead.
>  >  >
>  >  > src/pmgr/linux/common/dmm.c -
>  >
>  >  One of the goals of bridge is easier portability to other RTOS. You are
>  >  right about the additional layer to call into Linux OS. This was
>  >  intended to port this to other OS'es easily.
>  >
>
>  TI has to decide, what path they have to follow now onwards for
>  bridge, I see no need of OSAL layer, when we are targeting this as
>  Linux kernel driver.
>
>  It would be tough to get contributions as this code at first sight
>  looks harder to understand. Let's have some ideas on how we can clean
>  this code gradually without breaking API compatibility and IPC layer.
>  Common MMU and Mailbox framework written in Nokia dspgw should also be
>  utilized in this.
>

Forgot to mention that "documents" folder contains documents in MS$
.doc format. Please publish them in .pdf instead. Later the steps can
be converted to dspbridge-wiki pages.


-- 
---Trilok Soni
http://triloksoni.wordpress.com
--
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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux