On Fri, 16 Jun 2006, Takashi Iwai wrote:
> At Wed, 14 Jun 2006 16:14:17 +0200,
> I wrote:
> >
> > > What I am trying to say, is that your "simplification" of the API, has
> > > resulted in this sort of functionality being excluded, so I would need
> > > to implement yet another IOCTL to support it! My whole point of using
> > > the full request/response TLV api, was to not restrict us. I was also
> > > intending to maybe use the TLVs to set values, as well as just get
> > > values. For example, another way to implement the "continuous gain"
> > > feature would be to use the TLV api to set a flag, so that the 32bit
> > > values are returned via the current snd_ca0106_volume_get(), but only
> > > for this particular mixer open/close cycle, for backward compatibility
> > > with the current api.
> > >
> > > Do you prefer separate new IOCTLs for all these features?
> >
> > We can modify the implementation. For example, the patch like one
> > below. tlv_rw callback is completely free for own implementation.
> > Each callback is in change of reading/writing the given user-space
> > pointer arbitrarily.
>
> After reconsideration, I found it's better to split the ioctl.
> It's not nice that the caller has no idea whether the driver actually
> reads the value or not.
Yes, r/w ops should be splited (it's the reason why I used TLV_READ for
ioctl).
> Still I feel that the implementation of the callback is OK -- keep the
> core stuff as small and simple as possible, and let callbacks parse
> the user-pointer.
I have some cleanup in my tree to reduce memory usage. See bellow. Also,
while implementing, I realized that it might be worth to add
SNDRV_CTL_ELEM_ACCESS_TLV_READ and SNDRV_CTL_ELEM_ACCESS_TLV_WRITE flags
for control elements. Comments?
Jaroslav
diff -r 08577d0b45ef core/control.c
--- a/core/control.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/control.c Fri Jun 16 12:57:54 2006 +0200
@@ -241,7 +241,7 @@ struct snd_kcontrol *snd_ctl_new1(const
kctl.info = ncontrol->info;
kctl.get = ncontrol->get;
kctl.put = ncontrol->put;
- kctl.tlv = ncontrol->tlv;
+ kctl.tlv.p = ncontrol->tlv.p;
kctl.private_value = ncontrol->private_value;
kctl.private_data = private_data;
return snd_ctl_new(&kctl, access);
@@ -1068,8 +1068,9 @@ static int snd_ctl_subscribe_events(stru
return 0;
}
-static int snd_ctl_tlv_read(struct snd_card *card,
- struct snd_ctl_tlv __user *_tlv)
+static int snd_ctl_tlv_ioctl(struct snd_card *card,
+ struct snd_ctl_tlv __user *_tlv,
+ int write_flag)
{
struct snd_ctl_tlv tlv;
struct snd_kcontrol *kctl;
@@ -1086,17 +1087,26 @@ static int snd_ctl_tlv_read(struct snd_c
err = -ENOENT;
goto __kctl_end;
}
- if (kctl->tlv == NULL) {
- err = -ENXIO;
- goto __kctl_end;
- }
- len = kctl->tlv[1] + 2 * sizeof(unsigned int);
- if (tlv.length < len) {
- err = -ENOMEM;
- goto __kctl_end;
- }
- if (copy_to_user(_tlv->tlv, kctl->tlv, len))
- err = -EFAULT;
+ if (kctl->tlv.p == NULL) {
+ err = -ENXIO;
+ goto __kctl_end;
+ }
+ if (kctl->vd[tlv.numid - kctl->id.numid].access &
+ SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
+ err = kctl->tlv.c(kctl, write_flag, tlv.length, _tlv->tlv);
+ } else {
+ if (write_flag) {
+ err = -ENXIO;
+ goto __kctl_end;
+ }
+ len = kctl->tlv.p[1] + 2 * sizeof(unsigned int);
+ if (tlv.length < len) {
+ err = -ENOMEM;
+ goto __kctl_end;
+ }
+ if (copy_to_user(_tlv->tlv, kctl->tlv.p, len))
+ err = -EFAULT;
+ }
__kctl_end:
up_read(&card->controls_rwsem);
return err;
@@ -1141,7 +1151,9 @@ static long snd_ctl_ioctl(struct file *f
case SNDRV_CTL_IOCTL_SUBSCRIBE_EVENTS:
return snd_ctl_subscribe_events(ctl, ip);
case SNDRV_CTL_IOCTL_TLV_READ:
- return snd_ctl_tlv_read(card, argp);
+ return snd_ctl_tlv_ioctl(card, argp, 0);
+ case SNDRV_CTL_IOCTL_TLV_WRITE:
+ return snd_ctl_tlv_ioctl(card, argp, 1);
case SNDRV_CTL_IOCTL_POWER:
return -ENOPROTOOPT;
case SNDRV_CTL_IOCTL_POWER_STATE:
diff -r 08577d0b45ef include/asound.h
--- a/include/asound.h Tue Jun 13 12:01:14 2006 +0200
+++ b/include/asound.h Fri Jun 16 12:57:54 2006 +0200
@@ -731,6 +731,7 @@ typedef int __bitwise snd_ctl_elem_iface
#define SNDRV_CTL_ELEM_ACCESS_INACTIVE (1<<8) /* control does actually nothing, but may be updated */
#define SNDRV_CTL_ELEM_ACCESS_LOCK (1<<9) /* write lock */
#define SNDRV_CTL_ELEM_ACCESS_OWNER (1<<10) /* write lock owner */
+#define SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK (1<<28) /* kernel use a TLV callback */
#define SNDRV_CTL_ELEM_ACCESS_USER (1<<29) /* user space element */
#define SNDRV_CTL_ELEM_ACCESS_DINDIRECT (1<<30) /* indirect access for matrix dimensions in the info structure */
#define SNDRV_CTL_ELEM_ACCESS_INDIRECT (1<<31) /* indirect access for element value in the value structure */
@@ -838,6 +839,7 @@ enum {
SNDRV_CTL_IOCTL_ELEM_REPLACE = _IOWR('U', 0x18, struct snd_ctl_elem_info),
SNDRV_CTL_IOCTL_ELEM_REMOVE = _IOWR('U', 0x19, struct snd_ctl_elem_id),
SNDRV_CTL_IOCTL_TLV_READ = _IOWR('U', 0x1a, struct snd_ctl_tlv),
+ SNDRV_CTL_IOCTL_TLV_WRITE = _IOWR('U', 0x1b, struct snd_ctl_tlv),
SNDRV_CTL_IOCTL_HWDEP_NEXT_DEVICE = _IOWR('U', 0x20, int),
SNDRV_CTL_IOCTL_HWDEP_INFO = _IOR('U', 0x21, struct snd_hwdep_info),
SNDRV_CTL_IOCTL_PCM_NEXT_DEVICE = _IOR('U', 0x30, int),
diff -r 08577d0b45ef include/control.h
--- a/include/control.h Tue Jun 13 12:01:14 2006 +0200
+++ b/include/control.h Fri Jun 16 12:57:54 2006 +0200
@@ -30,6 +30,11 @@ typedef int (snd_kcontrol_info_t) (struc
typedef int (snd_kcontrol_info_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_info * uinfo);
typedef int (snd_kcontrol_get_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_value * ucontrol);
typedef int (snd_kcontrol_put_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_value * ucontrol);
+typedef int (snd_kcontrol_tlv_rw_t)(struct snd_kcontrol *kcontrol,
+ int write_flag,
+ unsigned int size,
+ unsigned int __user *tlv);
+
struct snd_kcontrol_new {
snd_ctl_elem_iface_t iface; /* interface identifier */
@@ -42,7 +47,10 @@ struct snd_kcontrol_new {
snd_kcontrol_info_t *info;
snd_kcontrol_get_t *get;
snd_kcontrol_put_t *put;
- unsigned int *tlv;
+ union {
+ snd_kcontrol_tlv_rw_t *c;
+ unsigned int *p;
+ } tlv;
unsigned long private_value;
};
@@ -59,7 +67,11 @@ struct snd_kcontrol {
snd_kcontrol_info_t *info;
snd_kcontrol_get_t *get;
snd_kcontrol_put_t *put;
- unsigned int *tlv;
+ snd_kcontrol_tlv_rw_t *tlv_rw;
+ union {
+ snd_kcontrol_tlv_rw_t *c;
+ unsigned int *p;
+ } tlv;
unsigned long private_value;
void *private_data;
void (*private_free)(struct snd_kcontrol *kcontrol);
-----
Jaroslav Kysela <perex@xxxxxxx>
Linux Kernel Sound Maintainer
ALSA Project, SUSE Labs
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/alsa-devel
