lkml.org 
[lkml]   [2021]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 5/7] ASoC: imx-pcm-rpmsg: Add platform driver for audio base on rpmsg
On Fri, Feb 05, 2021 at 02:57:28PM +0800, Shengjiu Wang wrote:

> + if (params_format(params) == SNDRV_PCM_FORMAT_S16_LE)
> + msg->s_msg.param.format = RPMSG_S16_LE;
> + else if (params_format(params) == SNDRV_PCM_FORMAT_S24_LE)

Again this should be a switch statement.

> + if (params_channels(params) == 1)
> + msg->s_msg.param.channels = RPMSG_CH_LEFT;
> + else
> + msg->s_msg.param.channels = RPMSG_CH_STEREO;

Shouldn't this be reporting an error if the number of channels is more
than 2?

> + /*
> + * if the data in the buffer is less than one period
> + * send message immediately.
> + * if there is more than one period data, delay one
> + * period (timer) to send the message.
> + */
> + if ((avail - writen_num * period_size) <= period_size) {
> + imx_rpmsg_insert_workqueue(substream, msg, info);
> + } else if (rpmsg->force_lpa && !timer_pending(timer)) {
> + int time_msec;
> +
> + time_msec = (int)(runtime->period_size * 1000 / runtime->rate);
> + mod_timer(timer, jiffies + msecs_to_jiffies(time_msec));
> + }

The comment here is at least confusing - why would we not send a full
buffer immediately if we have one? This sounds like it's the opposite
way round to what we'd do if we were trying to cut down the number of
messages. It might help to say which buffer and where?

> + /**
> + * Every work in the work queue, first we check if there

/** comments are only for kerneldoc.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2021-02-06 00:02    [W:0.119 / U:5.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site