lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 24 Nov 2006 12:41:48 +0100 From: Takashi Iwai <tiwai@...e.de> To: Pavel Machek <pavel@....cz> Cc: Tony Lindgren <tony@...mide.com>, kernel list <linux-kernel@...r.kernel.org>, Vladimir Ananiev <vovan888@...il.com>, linux-omap <linux-omap-open-source@...ux.omap.com> Subject: Re: sx1 mixer support At Fri, 24 Nov 2006 12:14:45 +0100, Pavel Machek wrote: > > (I think this should go in through omap tree, because sx1 support in > mainline is nonexistent for now, but...) Yes, it's fine with me. > --- /dev/null > +++ b/sound/arm/omap/omap-alsa-sx1-mixer.c (snip) > +static int current_volume = 0; /* current volume, we cant read it */ > +static int current_fm_volume = 0; /* current FM radio volume, we cant read it */ Zero-initializations can be stripped. (And lines seem still too long :) > +int set_mixer_volume(int mixer_vol) Missing static? Also there are many global functions around here. > +{ > + /* FIXME: Alsa has mixer_vol in 0-100 range, while SX1 needs 0-9 range */ An incorrect information. ALSA native mixers can use any value ranges while OSS mixers require 0-100 range. > +/* ---------------------------------------------------------------------------------------- */ > +static int __pcm_playback_target_info(snd_kcontrol_t *kcontrol, snd_ctl_elem_info_t *uinfo) What is the reason to use __ prefix? Don't use snd_xxx_t typedefs any more. They are obsolete (and will be removed in 2.6.20 tree). Also, the line is too long (ditto for other callbacks). > +static int __pcm_playback_target_put(snd_kcontrol_t *kcontrol, snd_ctl_elem_value_t *ucontrol) > +{ > + int ret_val = 0; > + int cur_val = ucontrol->value.integer.value[0]; > + > + if ((cur_val >= 0) && > + (cur_val < PLAYBACK_TARGET_COUNT) && > + (cur_val != current_playback_target)) { > + if (cur_val == PLAYBACK_TARGET_LOUDSPEAKER) { > + set_record_source(REC_SRC_SINGLE_ENDED_MICIN_HED); > + set_loudspeaker_to_playback_target(); > + } > + else if (cur_val == PLAYBACK_TARGET_HEADPHONE) { '}' ane else should be in the same line. > +/* > + * Alsa mixer interface function for getting the volume read from the SX1 in a > + * 0-100 alsa mixer format. > + */ I don't think this comment is correct... > +static int __pcm_playback_volume_get(snd_kcontrol_t *kcontrol, snd_ctl_elem_value_t *ucontrol) > +{ > + ucontrol->value.integer.value[0] = current_volume; > + return 0; > +} > + > +static int __pcm_playback_volume_put(snd_kcontrol_t *kcontrol, snd_ctl_elem_value_t *ucontrol) > +{ > + return set_mixer_volume(ucontrol->value.integer.value[0]); The put callback is supposed to return the following values: 0 = the value isn't changed 1 = the value is changed netagive value = error The return of 1 is not strictly needed in practice, but it's better to behave in a correct manner. > --- /dev/null > +++ b/sound/arm/omap/omap-alsa-sx1.c (snip) > +/* Send IPC message to sound server */ > +extern int cn_sx1snd_send(unsigned int cmd, unsigned int arg1, unsigned int arg2) > +{ > + struct cn_msg *m; > + unsigned short data[3]; > + int err; > + > + m = kzalloc(sizeof(*m) + sizeof(data), GFP_ATOMIC); Better to use gfp_any() here, too? (Or is it really too big for stack?) > --- /dev/null > +++ b/sound/arm/omap/omap-alsa-sx1.h (snip) > +/* > + * Defines codec specific functions pointers that can be used from the > + * common omap-alsa base driver for all omap codecs. > + */ > +void egold_configure(void); > +void egold_set_samplerate(long rate); > +void egold_clock_setup(void); > +int egold_clock_on(void); > +int egold_clock_off(void); > +int egold_get_default_samplerate(void); Do they need to be exported? I see them only in a single file as callbacks... Thanks, Takashi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists