[PATCH v8 16/22] ASoC: qdsp6: audioreach: add module configuration command helpers

[Srinivas Kandagatla]

On 27/09/2021 17:16, Pierre-Louis Bossart wrote:
> 
>> +static int audioreach_shmem_set_media_format(struct q6apm_graph *graph,
>> +					     struct audioreach_module *module,
>> +					     struct audioreach_module_config *mcfg)
>> +{
>> +	uint32_t num_channels = mcfg->num_channels;
>> +	struct apm_module_param_data *param_data;
>> +	struct payload_media_fmt_pcm *cfg;
>> +	struct media_format *header;
>> +	int rc, payload_size;
>> +	struct gpr_pkt *pkt;
>> +	void *p;
>> +
>> +	if (num_channels > 2) {
>> +		dev_err(graph->dev, "Error: Invalid channels (%d)!\n", num_channels);
>> +		return -EINVAL;
>> +	}
> 
> so here mcfg->num_channels > 2 is flagged as an error, but ...
> 
>> +
>> +	payload_size = APM_SHMEM_FMT_CFG_PSIZE(num_channels) + APM_MODULE_PARAM_DATA_SIZE;
>> +
>> +	pkt = audioreach_alloc_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0,
>> +				     graph->port->id, module->instance_id);
>> +	if (IS_ERR(pkt))
>> +		return PTR_ERR(pkt);
>> +
>> +	p = (void *)pkt + GPR_HDR_SIZE + APM_CMD_HDR_SIZE;
>> +
>> +	param_data = p;
>> +	param_data->module_instance_id = module->instance_id;
>> +	param_data->error_code = 0;
>> +	param_data->param_id = PARAM_ID_MEDIA_FORMAT;
>> +	param_data->param_size = payload_size - APM_MODULE_PARAM_DATA_SIZE;
>> +	p = p + APM_MODULE_PARAM_DATA_SIZE;
>> +
>> +	header = p;
>> +	header->data_format = DATA_FORMAT_FIXED_POINT;
>> +	header->fmt_id = MEDIA_FMT_ID_PCM;
>> +	header->payload_size = payload_size - sizeof(*header);
>> +
>> +	p = p + sizeof(*header);
>> +	cfg = p;
>> +	cfg->sample_rate = mcfg->sample_rate;
>> +	cfg->bit_width = mcfg->bit_width;
>> +	cfg->alignment = PCM_LSB_ALIGNED;
>> +	cfg->bits_per_sample = mcfg->bit_width;
>> +	cfg->q_factor = mcfg->bit_width - 1;
>> +	cfg->endianness = PCM_LITTLE_ENDIAN;
>> +	cfg->num_channels = mcfg->num_channels;
>> +
>> +	if (mcfg->num_channels == 1) {
>> +		cfg->channel_mapping[0] =  PCM_CHANNEL_L;
>> +	} else if (num_channels == 2) {
>> +		cfg->channel_mapping[0] =  PCM_CHANNEL_L;
>> +		cfg->channel_mapping[1] =  PCM_CHANNEL_R;
>> +	} else {
>> +		dev_err(graph->dev, "Error: Invalid channels (%d)!\n", num_channels);
>> +		rc = -EINVAL;
>> +		goto err;
> 
> ... this is again the case where mcfg->num_channels > 2 so this block is
> never executed.

I agree this else is dead code, will remove this.

--srini
> 
>> +	}
>> +
>> +	rc = audioreach_graph_send_cmd_sync(graph, pkt, 0);
>> +err:
>> +	kfree(pkt);
>> +
>> +	return rc;
>> +}
>> +