On 01/19/2016 06:00 PM, Russell King - ARM Linux wrote:
On Tue, Jan 19, 2016 at 02:40:32PM +0100, Arnaud Pouliquen wrote:
+static int snd_pcm_iec958_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *uctl)+{
- struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
- if (params->mutex)
mutex_unlock(params->mutex);- uctl->value.iec958.status[0] = params->iec->status[0];
- uctl->value.iec958.status[1] = params->iec->status[1];
- uctl->value.iec958.status[2] = params->iec->status[2];
- uctl->value.iec958.status[3] = params->iec->status[3];
- uctl->value.iec958.status[4] = params->iec->status[4];
- if (params->mutex)
mutex_unlock(params->mutex);I'm not sure it makes sense for the mutex to be optional.
I have no use case in mind that justifies optional mutex. Just did it for flexibility... I can suppress condition to force drivers to use it.
- return 0;
+}
+static int snd_pcm_iec958_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *uctl)+{
- struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
- int err = 0;
- if (params->mutex)
mutex_lock(params->mutex);- if (params->ctrl_set)
err = params->ctrl_set(params->pdata,uctl->value.iec958.status, 5);- if (!err) {
params->iec->status[0] = uctl->value.iec958.status[0];params->iec->status[1] = uctl->value.iec958.status[1];params->iec->status[2] = uctl->value.iec958.status[2];params->iec->status[3] = uctl->value.iec958.status[3];params->iec->status[4] = uctl->value.iec958.status[4];- }
- if (params->mutex)
mutex_unlock(params->mutex);- return 0;
I think you're supposed to report whether anything changed, rather than just zero on success.
right, need to return 1 or err
+}
+static const struct snd_kcontrol_new iec958_ctls[] = {
- {
.iface = SNDRV_CTL_ELEM_IFACE_PCM,.name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),Shouldn't this have a .access member? What if the audio driver modifies the IEC958 status for PCM playback - shouldn't it have the ability to have SNDRV_CTL_ELEM_ACCESS_VOLATILE added?
i will add READ + VOLATILE access for capture, RW +VOLATILE access for playback. Should i also add some other controls by default, like controls for consumer and professional mask for playback?