Re: [Sound-open-firmware] [PATCH] volume: ctrl cmd id check and prepare fix

On Mon, 2017-09-11 at 22:21 +0100, Ranjani Sridharan wrote:

This patch fixes the following:

a. source_period_bytes/sink_period_bytes calculation fix with the correct frame_bytes b. check if the ctrl cmd is for the volume before processing it

Could you do this as 2 patches. One minor comment below too.

Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com

src/audio/volume.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/audio/volume.c b/src/audio/volume.c index 5808bed..82b8848 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -471,9 +471,17 @@ static int volume_cmd(struct comp_dev *dev, int cmd, void *data)

switch (cmd) { case COMP_CMD_SET_VALUE:

• return volume_ctrl_set_cmd(dev, cdata);
  

• /* check if the ctrl cmd is for this component */
  

• if ((cdata != NULL) && (cdata->comp_id != dev->comp.id))
  

• 	return 0;
  

We should return an error if cdata is NULL or there is an ID mismatch. These can also both be check once at the function entry rather than in the switch statement.

Liam

• else
  

• 	return volume_ctrl_set_cmd(dev, cdata);
  

  case COMP_CMD_GET_VALUE:

• return volume_ctrl_get_cmd(dev, cdata);
  

• /* check if the ctrl cmd is for this component */
  

• if ((cdata != NULL) && (cdata->comp_id != dev->comp.id))
  

• 	return 0;
  

• else
  

• 	return volume_ctrl_get_cmd(dev, cdata);
  

  case COMP_CMD_START: case COMP_CMD_STOP: case COMP_CMD_PAUSE:

@@ -555,7 +563,7 @@ static int volume_prepare(struct comp_dev *dev) sconfig = COMP_GET_CONFIG(sourceb->source); cd->source_format = sconfig->frame_fmt; cd->source_period_bytes = dev->frames *

• 	sourceb->source->frame_bytes;
  

• 	comp_frame_bytes(sourceb->source);
  

  break; }

@@ -575,7 +583,7 @@ static int volume_prepare(struct comp_dev *dev) sconfig = COMP_GET_CONFIG(sinkb->sink); cd->sink_format = sconfig->frame_fmt; cd->sink_period_bytes = dev->frames *

• 	sinkb->sink->frame_bytes;
  

• 	comp_frame_bytes(sinkb->sink);
  

  break; }