On Aug 30 2016 23:51, Vinod Koul wrote:
On Tue, Aug 30, 2016 at 04:09:33PM +0900, Takashi Sakamoto wrote:
On Aug 30 2016 16:05, Clemens Ladisch wrote:
Takashi Sakamoto wrote:
[...] APIs return operated length, while TLV feature can't. This is inconvenient to applications.
This commit enables control core to return operated length of TLV feature. This changes the prototype of 'snd_kcontrol_tlv_rw_t' to get a pointer to size variable so that each implementation of the prototype can modify the variable with operated length.
I'll use this function as an example:
--- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -535,17 +535,20 @@ int snd_usb_set_cur_mix_value(struct usb_mixer_elem_info *cval, int channel,
- TLV callback for mixer volume controls
*/ int snd_usb_mixer_vol_tlv(struct snd_kcontrol *kcontrol, int op_flag,
unsigned int size, unsigned int __user *_tlv)
unsigned int *size, unsigned int __user *_tlv){ struct usb_mixer_elem_info *cval = kcontrol->private_data; DECLARE_TLV_DB_MINMAX(scale, 0, 0);
- if (size < sizeof(scale))
- if (*size < sizeof(scale)) return -ENOMEM; scale[2] = cval->dBmin; scale[3] = cval->dBmax; if (copy_to_user(_tlv, scale, sizeof(scale))) return -EFAULT;
- *size = sizeof(scale);
- return 0;
}
The size is already returned in scale[1] (it's initialized by DECLARE_TLV_DB_MINMAX()). That's exactly what the "L" in "TLV" means.
All other TLV callbacks also take care to set this field correctly.
If there were any TLV callback that did not set _tlv[1] to the actual size, it would be buggy, and just needed to be fixed to do so.
As I described, TLV feature of ALSA control interface is not only used to transfer threshold level information, but also arbitrary data for I/O by developers in ALSA SoC part. The '_tlv[1]' protocol is not necessarily kept by them.
can you explain what you mean by 'to transfer threshold level information'
And on this discussion, IIUC, we should fill tlv[1] with size being returned right? For the asoc part, I think we should fix snd_soc_bytes_tlv_callback() to update this.
The layout of TLV packet is: struct snd_ctl_tlv { unsigned int numid; # numerical ID of a control element unsigned int length; # length of payload unsigned int tlv[0]; # payload }; http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/include/uap...
In our implementaion, TLV packet payload (struct snd_ctl_tlv.tlv) is used to transfer data. For pure threshold level information, we expects applications and drivers to fill the payload with this protocol: struct snd_ctl_tlv.tlv[0]: one of SNDRV_CTL_TLVT_XXX struct snd_ctl_tlv.tlv[1]: length of data struct snd_ctl_tlv.tlv[2..]: data
(You can see SNDRV_CTL_TLVT_XXX in this header. http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/include/uap... )
On the other hand, ALSA SoC part performs: struct snd_ctl_tlv.tlv[0..]: arbitrary data
If your 'tlv[1]' means the 'struct snd_ctl_tlv.tlv[1]', no sense.
The issue I address is current implementation cannot correctly handle this case: - applications request a buffer with a certain size - drivers processes the request with smaller size - application cannot get the size
When implementing I/O operation, in my understanding, this situation often occurs, depending on driver implementation. Fortunately, current implementation of WM-ADSP is free from this concern, but it's better for us not to expect this luck always.
Regards
Takashi Sakamoto