Re: OMAP3 DSPBridge review

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

 



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.

-- 
---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