At Fri, 3 Jul 2009 11:36:03 +0530,
Koul, Vinod wrote:
>>> Following emails are a of series 13 patches for Intel SST drivers
> (including documentation). These drivers are for the audio
> engine/sound card in upcoming Intel based Mobile Internet Devices
> (MID) platform
>> There are 2 drivers involved. One is for the audio DSP engine that
> can decode and do audio processing and one is for the ALSA sound
> card driver with support for three different sound cards that are
> supported on the platform.
>> The dependant drivers for the platform are not yet available in the
> upstream. So please consider these patches only for RFC and provide
> your comments/suggestions.
>> The audio firmware binary associated with this platform that is
> downloaded by the driver at runtime will be shared in a public
> location and made available in firmware git when we release the
> drivers for up streaming.
>> The patches are compiled with 0 warnings and checkpatch-ed.
>> The first patch in the series [RFC 1/13] Intel SST driver's
> documentation gives the documentation about the drivers and the
> interface for the audio DSP driver. The rest of the patches that
> follow have a brief description about the contents.
First off, thanks for patches!
They are lengthy, so I just took a quick glance over them without
reading deeply inside. Some comments have been already sent back.
In general, please fix the following:
- Check whether each function is really global. Make functions static
as much as possible.
This improves not only the compiler optimization but also the
readability. (Without static, you'll have to remember that this
function can be called from the outside.)
- Put blank lines appropriately. Sometimes hard to read too stuffed
texts.
- Avoid unnecessary variable initializations at the beginning of
functions. I know there are old school's text books of C
recommending such a thing. But, it's bad. It just hides possible
logical mistakes. Modern compilers would warn anyway uninitialized
variables.
- Avoid the expression like if (0 == foo). This is annoying because
the expression is illogical. We compare foo with 0, not vice
versa.
I know this as yet another bad habit by some C books (to avoid a
mistake like if (foo = 0)), but such a mistake would be, again,
warned by a compiler. So, don't use tricks.
HTH,
Takashi