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

[Ranjani Sridharan]

Thanks, Liam. I'll split up the patch into 2. 

Also, just a comment that it should not return an error when there is
an ID mismatch. It just means that the command is not meant for this
component and it should just pass it forward. I will change it to
return an error if cdata is NULL. 



On Tue, 2017-09-12 at 12:36 +0100, Liam Girdwood wrote:
> 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 at 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;
> >  	}
> >  
> 
> 
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware