At Fri, 3 Jul 2009 12:33:32 +0530,
Vinod Koul wrote:
>
> This patch adds header files private to the SST
> driver and contains the common structures like driver context,
> DSP register definitions, and inline utility
> functions used by the SST driver and function prototypes of
> common functions that are implemented in SST driver
>
> Signed-off-by: Vinod Koul <vinod.koul@xxxxxxxxx>
> Signed-off-by: Harsha Priya <priya.harsha@xxxxxxxxx>
>
> new file: sound/pci/sst/intel_sst_common.h
> new file: sound/pci/sst/intel_sst_pvt.h
BTW, please don't add comments after sing-off.
If any, write below '---' line so that git will ignore it.
> ---
> sound/pci/sst/intel_sst_common.h | 292 +++++++++++++++++++++++++++++
> sound/pci/sst/intel_sst_pvt.h | 378 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 670 insertions(+), 0 deletions(-)
> create mode 100644 sound/pci/sst/intel_sst_common.h
> create mode 100644 sound/pci/sst/intel_sst_pvt.h
>
> diff --git a/sound/pci/sst/intel_sst_common.h b/sound/pci/sst/intel_sst_common.h
> new file mode 100644
> index 0000000..e039489
> --- /dev/null
> +++ b/sound/pci/sst/intel_sst_common.h
> +/*
> +SST shim registers to structure mapping
> +*/
> +union config_status_reg {
> + struct {
> + u32 rsvd0:1;
> + u32 sst_reset:1;
> + u32 hw_rsvd:3;
> + u32 sst_clk:2;
> + u32 bypass:3;
> + u32 run_stall:1;
> + u32 rsvd:21;
> + } part;
> + u32 full;
A union with bit fields is very bad for portability.
If it's used as a communication parameter, don't use it.
Just pass u32 and get/set via normal bit and/or operations.
> +struct snd_sst_user_cap_list {
> + unsigned int iov_index; /*index of iov*/
> + unsigned long iov_offset; /*offset in iov*/
> + unsigned long offset; /*offset in kmem*/
> + unsigned long size; /*size copied*/
Sure to use long here?
> diff --git a/sound/pci/sst/intel_sst_pvt.h b/sound/pci/sst/intel_sst_pvt.h
> new file mode 100644
> index 0000000..a1e786e
> --- /dev/null
> +++ b/sound/pci/sst/intel_sst_pvt.h
(snip)
> +static inline int sst_scu_ipc_write(u32 addr, u32 value)
> +{
What this must be inline?
> +#ifndef CONFIG_SST_IPC_NOT_INCLUDED
> + int retval = 0, retry = 3;
> + struct ipc_reg_data ipc_reg = {0};
> +
> + ipc_reg.address = addr;
> + ipc_reg.data = value;
> + ipc_reg.ioc = 1;
> +
> + while (retry) {
> + retval = mrst_ipc_write32(&ipc_reg);
> + if (retval == 0)
> + break;
> + retry--;
> + /*error*/
> + sst_err("IPC write failed %x\n", retval);
> + }
> + return retval;
> +#else
> + return 0;
> +#endif
> +}
> +static inline void sst_fill_header(union ipc_header *header,
> + int msg, int large, int strID)
> +{
> + header->part.msg_id = msg;
> + header->part.str_id = strID;
> + header->part.large = large;
> + header->part.done = 0;
> + header->part.busy = 1;
> + header->part.data = 0;
> +}
> +
> +static inline int sst_get_stream_id(struct intel_sst_drv *sst_ops, int pvt_id)
> +{
> + int i;
> +
> + for (i = 1; i < MAX_NUM_STREAMS; i++) {
> + if (pvt_id == sst_ops->streams[i].sst_id)
> + return i;
> + }
> + return -EINVAL;
> +}
> +
> +static inline unsigned int sst_assign_pvt_id(struct intel_sst_drv *sst_ops)
> +{
> + sst_ops->unique_id++;
> + if (sst_ops->unique_id >= MAX_NUM_STREAMS)
> + sst_ops->unique_id = 1;
> + return sst_ops->unique_id;
> +}
> +
> +static inline int sst_get_block_stream(struct intel_sst_drv *sst_ops)
This looks too big for inline.
> +{
> + int i;
> +
> + for (i = 0; i < MAX_ACTIVE_STREAM; i++) {
> + if (BLOCK_UNINIT == sst_ops->alloc_block[i].sst_id) {
> + sst_ops->alloc_block[i].ops_block.condition = false;
> + sst_ops->alloc_block[i].ops_block.ret_code = 0;
> + sst_ops->alloc_block[i].sst_id = 0;
> + break;
> + }
> + }
> + if (MAX_ACTIVE_STREAM == i) {
> + sst_err("max alloc_stream reached \n");
> + i = -EBUSY; /*active stream limit reached*/
> + }
> + return i;
> +}
> +
> +
> +
> +static inline void sst_print_hex(unsigned char *buf, unsigned int size)
And this doesn't have to be.
> +{
> + unsigned int i;
> +
> + for (i = 0; i < size; i++) {
> + sst_dbg("%02x ", buf[i]);
> + if ((i != 0) && ((i % 8) == 0))
> + sst_dbg("\n");
> + }
> +}
> +
> +static inline void sst_init_stream(struct stream_info *stream,
> + int codec, int sst_id, int ops)
> +{
> + stream->status = STREAM_INIT;
> + stream->prev = STREAM_UN_INIT;
> + stream->codec = codec;
> + stream->sst_id = sst_id;
> + stream->ops = ops;
> + stream->data_blk.on = false;
> + stream->data_blk.condition = false;
> + stream->data_blk.ret_code = 0;
> + stream->data_blk.data = NULL;
> + stream->ctrl_blk.on = false;
> + stream->ctrl_blk.condition = false;
> + stream->ctrl_blk.ret_code = 0;
> + stream->ctrl_blk.data = NULL;
> + stream->mmapped = false;
> +}
> +
> +static inline void sst_clean_stream(struct stream_info *stream)
> +{
> + struct sst_stream_bufs *bufs = NULL, *_bufs = NULL;
> + stream->status = STREAM_UN_INIT;
> + stream->prev = STREAM_UN_INIT;
> + list_for_each_entry_safe(bufs, _bufs, &stream->bufs, node) {
> + list_del(&bufs->node);
> + kfree(bufs);
> + }
> +}
> +
> +static inline int sst_wait_interruptible(struct intel_sst_drv *sst_ops,
> + struct sst_block *block)
> +{
> + int retval = 0;
> +
> + if (!wait_event_interruptible(sst_ops->wait_queue, block->condition)) {
> + /*event wake*/
> + if (0 != block->ret_code) {
> + sst_err("stream failed %d\n", block->ret_code);
> + retval = -EBUSY;
> + } else {
> + sst_dbg("event up\n");
> + retval = 0;
> + }
> + } else {
> + sst_err("signal interrupted\n");
> + retval = -EINTR;
> + }
> + return retval;
> +
> +}
> +
> +static inline int sst_wait_interruptible_timeout(struct intel_sst_drv *sst_ops,
> + struct sst_block *block, int timeout)
> +{
> + int retval = 0;
> +
> + sst_dbg("waiting....\n");
> + if (wait_event_interruptible_timeout(sst_ops->wait_queue,
> + block->condition,
> + msecs_to_jiffies(timeout))) {
> + /*event wake*/
> + sst_dbg("Event wake ...\n");
> + if (0 != block->ret_code)
> + sst_err("stream failed %d\n", block->ret_code);
> + else
> + sst_dbg("event up\n");
> + retval = block->ret_code;
> + } else {
> + sst_err("timeout occured...\n");
> + retval = -EBUSY;
> + }
> + return retval;
> +
> +}
> +
> +static inline int sst_wait_timeout(struct intel_sst_drv *sst_ops,
> + struct stream_alloc_block *block)
> +{
> + int retval = 0;
> +
> + /*NOTE:
> + Observed that FW processes the alloc msg and replies even
> + before the alloc thread has finished execution*/
> + sst_dbg("waiting for %x, condition %x \n", block->sst_id,
> + block->ops_block.condition);
> + if (wait_event_interruptible_timeout(sst_ops->wait_queue,
> + block->ops_block.condition,
> + msecs_to_jiffies(SST_BLOCK_TIMEOUT))) {
> + /*event wake*/
> + sst_dbg("Event wake ... %x \n", block->ops_block.condition);
> + /*check return*/
> + if (0 > block->ops_block.ret_code)
> + sst_err("blk failed %d\n", block->ops_block.ret_code);
> + else
> + sst_dbg("blk ret: %d\n", block->ops_block.ret_code);
> + retval = block->ops_block.ret_code;
> + } else {
> + sst_err("Wait timed-out %x\n", block->ops_block.condition);
> + retval = -EBUSY;
> + }
> + return retval;
> +
> +}
> +
> +
> +static inline int sst_create_large_msg(struct ipc_post **arg)
> +{
> + struct ipc_post *msg;
> +
> + msg = kzalloc(sizeof(struct ipc_post), GFP_ATOMIC);
> + if (NULL == msg) {
> + sst_err("kzalloc msg failed \n");
> + return -ENOMEM;
> + }
> +
> + msg->mailbox_data = kzalloc(SST_MAILBOX_SIZE, GFP_ATOMIC);
> + if (NULL == msg->mailbox_data) {
> + kfree(msg);
> + sst_err("kzalloc mailbox_data failed \n");
> + return -ENOMEM;
> + };
> + *arg = msg;
> + return 0;
> +}
> +
> +static inline int sst_create_short_msg(struct ipc_post **arg)
> +{
> + struct ipc_post *msg;
> +
> + msg = kzalloc(sizeof(*msg), GFP_ATOMIC);
> + if (NULL == msg) {
> + sst_err("kzalloc msg failed \n");
> + return -ENOMEM;
> + }
> + msg->mailbox_data = NULL;
> + *arg = msg;
> + return 0;
> +}
> +
> +static inline void sst_print_params(struct snd_sst_stream_params *sparam)
> +{
> + switch (sparam->uc.pcm_params.codec) {
> + case SST_CODEC_TYPE_PCM:
> + sst_dbg("pcm \n");
> + sst_dbg("chan=%d, sfreq = %d, wd_sz = %d \
> + brate = %d framesize = %d \
> + samples_per_frame = %d \
> + period_cnt = %d\n",
> + sparam->uc.pcm_params.num_chan,
> + sparam->uc.pcm_params.sfreq,
> + sparam->uc.pcm_params.pcm_wd_sz,
> + sparam->uc.pcm_params.brate,
> + sparam->uc.pcm_params.frame_size,
> + sparam->uc.pcm_params.samples_per_frame,
> + sparam->uc.pcm_params.period_count);
> + break;
> +
> + case SST_CODEC_TYPE_MP3:
> + sst_dbg("mp3 \n");
> + sst_dbg("chan=%d, brate=%d, sfreq = %d, wd_sz = %d\n",
> + sparam->uc.mp3_params.num_chan,
> + sparam->uc.mp3_params.brate,
> + sparam->uc.mp3_params.sfreq,
> + sparam->uc.mp3_params.pcm_wd_sz);
> + break;
> +
> + case SST_CODEC_TYPE_AAC:
> + sst_dbg("aac \n");
> + sst_dbg("chan=%d, brate=%d, sfreq = %d, wd_sz = %d,asrate=%d\n",
> + sparam->uc.aac_params.num_chan,
> + sparam->uc.aac_params.brate,
> + sparam->uc.aac_params.sfreq,
> + sparam->uc.aac_params.pcm_wd_sz,
> + sparam->uc.aac_params.aac_srate);
> + sst_dbg("mpgid=%d profile=%d, aot = %d\n",
> + sparam->uc.aac_params.mpg_id,
> + sparam->uc.aac_params.aac_profile,
> + sparam->uc.aac_params.aot);
> + break;
> + case SST_CODEC_TYPE_WMA9:
> + sst_dbg("wma type \n");
> + sst_dbg("chan=%d, brate=%d, sfreq = %d, wd_sz = %d, tag=%d\n",
> + sparam->uc.wma_params.num_chan,
> + sparam->uc.wma_params.brate,
> + sparam->uc.wma_params.sfreq,
> + sparam->uc.wma_params.pcm_wd_sz,
> + sparam->uc.wma_params.format_tag);
> + sst_dbg("mask=%d, b align=%d, enc opt =%d, op align =%d\n",
> + sparam->uc.wma_params.channel_mask,
> + sparam->uc.wma_params.block_align,
> + sparam->uc.wma_params.wma_encode_opt,
> + sparam->uc.wma_params.op_align);
> + break;
> + default:
> + sst_dbg("other codec 0x%x\n", sparam->uc.pcm_params.codec);
> + }
> +}
> +static inline int sst_validate_strid(int str_id)
> +{
> + if (str_id <= 0 || str_id >= MAX_NUM_STREAMS) {
> + sst_err("invalid stream id \n");
> + return -EINVAL;
> + } else
> + return 0;
> +}
> +
> +/*NOTE: status will +ve for good cases and -ve for error ones*/
> +#define MAX_STREAM_FIELD 255
> +static inline void sst_wake_up_alloc_block(struct intel_sst_drv *sst_ops,
> + u8 sst_id, int status, void *data)
> +{
> + int i;
> +
> + /*Unblock with retval code*/
> + for (i = 0; i < MAX_STREAM_FIELD; i++) {
> + if (sst_id == sst_ops->alloc_block[i].sst_id) {
> + sst_ops->alloc_block[i].ops_block.condition = true;
> + sst_ops->alloc_block[i].ops_block.ret_code = status;
> + sst_ops->alloc_block[i].ops_block.data = data;
> + sst_dbg("wake id %d, sst_id %d condition %x\n", i,
> + sst_ops->alloc_block[i].sst_id,
> + sst_ops->alloc_block[i].ops_block.condition);
> + wake_up(&sst_ops->wait_queue);
> + break;
> + }
> + }
> +}
Well, all these shouldn't be inline if you have no concrete reason
to inline them. It's no C++ template. Better to put as normal functions.
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
