Re: [PATCH v5 1/2] ALSA: FCP: Add Focusrite Control Protocol driver

[Jaroslav Kysela <perex@xxxxxxxx>]

On 13. 01. 25 18:02, Takashi Iwai wrote:

> > +static int fcp_meter_tlv_callback(struct snd_kcontrol *kctl,
> > +				  int op_flag, unsigned int size,
> > +				  unsigned int __user *tlv)
> > +{
> > +	struct usb_mixer_elem_info *elem = kctl->private_data;
> > +	struct usb_mixer_interface *mixer = elem->head.mixer;
> > +	struct fcp_data *private = mixer->private_data;
> > +
> > +	if (op_flag == SNDRV_CTL_TLV_OP_READ) {
> > +		if (private->meter_labels_size == 0)
> > +			return 0;
> > +
> > +		if (size > private->meter_labels_size)
> > +			size = private->meter_labels_size;
> > +
> > +		if (copy_to_user(tlv, private->meter_labels, size))
> > +			return -EFAULT;
> > +
> > +		return size;
> > +	}
> > +
> > +	if (op_flag == SNDRV_CTL_TLV_OP_WRITE) {
> > +		kfree(private->meter_labels);
> > +		private->meter_labels = NULL;
> > +		private->meter_labels_size = 0;
> > +
> > +		if (size == 0)
> > +			return 0;
> > +
> > +		if (size > 4096)
> > +			return -EINVAL;
> > +
> > +		private->meter_labels = kmalloc(size, GFP_KERNEL);
> > +		if (!private->meter_labels)
> > +			return -ENOMEM;
> > +
> > +		if (copy_from_user(private->meter_labels, tlv, size)) {
> > +			kfree(private->meter_labels);
> > +			private->meter_labels = NULL;
> > +			return -EFAULT;
> > +		}
> > +
> > +		private->meter_labels_size = size;
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
>
> Hmm, this special is a special use of TLV in non-standard way, which
> needs definitely documentation.  The use is no longer TLV, just some
> raw read/write of a bulk data for the kcontrol , after all.

The data format _must_ be in TLV encapsulation also for such R/W operations. 
So, a new type should be defined. Perhaps, we can define a driver specific 
data type, because the meter levels (and the whole extensions) seem as device 
specific.

						Jaroslav

--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.