On 27/04/2022 13:36, Sergey Senozhatsky wrote:
> On (22/04/27 11:50), Peter Ujfalusi wrote:
>> @@ -784,16 +785,23 @@ static int sof_get_control_data(struct snd_soc_component *scomp,
>> }
>>
>> cdata = wdata[i].control->ipc_control_data;
>> - wdata[i].pdata = cdata->data;
>> - if (!wdata[i].pdata)
>> - return -EINVAL;
>>
>> /* make sure data is valid - data can be updated at runtime */
>> - if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES &&
>> - wdata[i].pdata->magic != SOF_ABI_MAGIC)
>> - return -EINVAL;
>> + if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES) {
>> + if (!cdata->data)
>> + return -EINVAL;
>> +
>> + if (cdata->data->magic != SOF_ABI_MAGIC)
>> + return -EINVAL;
>> +
>> + wdata[i].pdata = cdata->data->data;
>> + wdata[i].pdata_size = cdata->data->size;
>> + } else {
>> + wdata[i].pdata = cdata->chanv; /* points to the control data union */
>> + wdata[i].pdata_size = wdata[i].control->size;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> A question, so here wdata[i].control->size is
>
> scontrol->size = struct_size(scontrol->control_data, chanv,
> le32_to_cpu(mc->num_channels));
>
> Right?
>
> If so, then is this really what we memcpy()? We memcpy() control->data->chanv
> and its size is `sizeof(chanv) * le32_to_cpu(mc->num_channels)`, isn't it?
>
> [..]
>> if (ipc_data_size) {
>> for (i = 0; i < widget->num_kcontrols; i++) {
>> + if (!wdata[i].pdata_size)
>> + continue;
>> +
>> + memcpy(&process->data[offset], wdata[i].pdata,
>> + wdata[i].pdata_size);
>> + offset += wdata[i].pdata_size;
>> }
>> }
>
> So should sof_get_control_data() have instead of this
>
> wdata[i].pdata_size = wdata[i].control->size;
>
> something like this
>
> wdata[i].pdata_size = wdata[i].control->size - sizeof(struct sof_ipc_ctrl_data);
Yes, you are right.
>
> So that we will copy payload data, which is a bunch of chanv structs 8
> bytes each.
--
Péter
