[alsa-devel] [PATCH] es18xx: code improvements
Takashi Iwai
tiwai at suse.de
Sun Nov 8 11:27:03 CET 2009
At Sun, 8 Nov 2009 11:58:08 +0100,
Krzysztof Helt wrote:
>
> From: Krzysztof Helt <krzysztof.h1 at wp.pl>
>
> 1. Set the third argument of the snd_device_new to not NULL, so there is
> no warning about bug during chip detection. The third argument is not
> used in this driver. It was changed in my previous patch.
>
> 2. Remove the fm_port and mpu_port fields from the snd_es18xx structure.
> They can be converted to function arguments.
>
> 3. Remove the dmaN_size fields from the snd_es18xx structure. These
> values are used only in pointer functions and can be easily calculated.
>
> 4. Remove the ctrl_lock spinlock which is used only in one read function
> which is called once during chip initialization. There are many
> writes to the same register and they are not protected on purpose
> (see the comment ina the snd_es18xx_config_write()).
>
> 5. Use the first part of the text5Sources string table as the text4Soruces
> table (they are the same).
>
> 6. Merge the same cases for the ES1887 and ES1888 when setting chip's caps.
>
> 7. Move the snd_es18xx_reset() to __devinit section.
>
> Signed-off-by: Krzysztof Helt <krzysztof.h1 at wp.pl>
Looks good. Applied now.
Thanks!
Takashi
> ---
> sound/isa/es18xx.c | 91 +++++++++++++++++++++++----------------------------
> 1 files changed, 41 insertions(+), 50 deletions(-)
>
> diff --git a/sound/isa/es18xx.c b/sound/isa/es18xx.c
> index 5cf42b4..06e871e 100644
> --- a/sound/isa/es18xx.c
> +++ b/sound/isa/es18xx.c
> @@ -102,8 +102,6 @@
>
> struct snd_es18xx {
> unsigned long port; /* port of ESS chip */
> - unsigned long mpu_port; /* MPU-401 port of ESS chip */
> - unsigned long fm_port; /* FM port */
> unsigned long ctrl_port; /* Control port of ESS chip */
> struct resource *res_port;
> struct resource *res_mpu_port;
> @@ -116,8 +114,6 @@ struct snd_es18xx {
> unsigned short audio2_vol; /* volume level of audio2 */
>
> unsigned short active; /* active channel mask */
> - unsigned int dma1_size;
> - unsigned int dma2_size;
> unsigned int dma1_shift;
> unsigned int dma2_shift;
>
> @@ -135,7 +131,6 @@ struct snd_es18xx {
>
> spinlock_t reg_lock;
> spinlock_t mixer_lock;
> - spinlock_t ctrl_lock;
> #ifdef CONFIG_PM
> unsigned char pm_reg;
> #endif
> @@ -354,7 +349,7 @@ static inline int snd_es18xx_mixer_writable(struct snd_es18xx *chip, unsigned ch
> }
>
>
> -static int snd_es18xx_reset(struct snd_es18xx *chip)
> +static int __devinit snd_es18xx_reset(struct snd_es18xx *chip)
> {
> int i;
> outb(0x03, chip->port + 0x06);
> @@ -490,8 +485,6 @@ static int snd_es18xx_playback1_prepare(struct snd_es18xx *chip,
> unsigned int size = snd_pcm_lib_buffer_bytes(substream);
> unsigned int count = snd_pcm_lib_period_bytes(substream);
>
> - chip->dma2_size = size;
> -
> snd_es18xx_rate_set(chip, substream, DAC2);
>
> /* Transfer Count Reload */
> @@ -591,8 +584,6 @@ static int snd_es18xx_capture_prepare(struct snd_pcm_substream *substream)
> unsigned int size = snd_pcm_lib_buffer_bytes(substream);
> unsigned int count = snd_pcm_lib_period_bytes(substream);
>
> - chip->dma1_size = size;
> -
> snd_es18xx_reset_fifo(chip);
>
> /* Set stereo/mono */
> @@ -659,8 +650,6 @@ static int snd_es18xx_playback2_prepare(struct snd_es18xx *chip,
> unsigned int size = snd_pcm_lib_buffer_bytes(substream);
> unsigned int count = snd_pcm_lib_period_bytes(substream);
>
> - chip->dma1_size = size;
> -
> snd_es18xx_reset_fifo(chip);
>
> /* Set stereo/mono */
> @@ -821,17 +810,18 @@ static irqreturn_t snd_es18xx_interrupt(int irq, void *dev_id)
> static snd_pcm_uframes_t snd_es18xx_playback_pointer(struct snd_pcm_substream *substream)
> {
> struct snd_es18xx *chip = snd_pcm_substream_chip(substream);
> + unsigned int size = snd_pcm_lib_buffer_bytes(substream);
> int pos;
>
> if (substream->number == 0 && (chip->caps & ES18XX_PCM2)) {
> if (!(chip->active & DAC2))
> return 0;
> - pos = snd_dma_pointer(chip->dma2, chip->dma2_size);
> + pos = snd_dma_pointer(chip->dma2, size);
> return pos >> chip->dma2_shift;
> } else {
> if (!(chip->active & DAC1))
> return 0;
> - pos = snd_dma_pointer(chip->dma1, chip->dma1_size);
> + pos = snd_dma_pointer(chip->dma1, size);
> return pos >> chip->dma1_shift;
> }
> }
> @@ -839,11 +829,12 @@ static snd_pcm_uframes_t snd_es18xx_playback_pointer(struct snd_pcm_substream *s
> static snd_pcm_uframes_t snd_es18xx_capture_pointer(struct snd_pcm_substream *substream)
> {
> struct snd_es18xx *chip = snd_pcm_substream_chip(substream);
> + unsigned int size = snd_pcm_lib_buffer_bytes(substream);
> int pos;
>
> if (!(chip->active & ADC1))
> return 0;
> - pos = snd_dma_pointer(chip->dma1, chip->dma1_size);
> + pos = snd_dma_pointer(chip->dma1, size);
> return pos >> chip->dma1_shift;
> }
>
> @@ -974,9 +965,6 @@ static int snd_es18xx_capture_close(struct snd_pcm_substream *substream)
>
> static int snd_es18xx_info_mux(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo)
> {
> - static char *texts4Source[4] = {
> - "Mic", "CD", "Line", "Master"
> - };
> static char *texts5Source[5] = {
> "Mic", "CD", "Line", "Master", "Mix"
> };
> @@ -994,7 +982,8 @@ static int snd_es18xx_info_mux(struct snd_kcontrol *kcontrol, struct snd_ctl_ele
> uinfo->value.enumerated.items = 4;
> if (uinfo->value.enumerated.item > 3)
> uinfo->value.enumerated.item = 3;
> - strcpy(uinfo->value.enumerated.name, texts4Source[uinfo->value.enumerated.item]);
> + strcpy(uinfo->value.enumerated.name,
> + texts5Source[uinfo->value.enumerated.item]);
> break;
> case 0x1887:
> case 0x1888:
> @@ -1378,11 +1367,9 @@ ES18XX_SINGLE("Hardware Master Volume Split", 0, 0x64, 7, 1, 0),
> static int __devinit snd_es18xx_config_read(struct snd_es18xx *chip, unsigned char reg)
> {
> int data;
> - unsigned long flags;
> - spin_lock_irqsave(&chip->ctrl_lock, flags);
> +
> outb(reg, chip->ctrl_port);
> data = inb(chip->ctrl_port + 1);
> - spin_unlock_irqrestore(&chip->ctrl_lock, flags);
> return data;
> }
>
> @@ -1398,7 +1385,9 @@ static void __devinit snd_es18xx_config_write(struct snd_es18xx *chip,
> #endif
> }
>
> -static int __devinit snd_es18xx_initialize(struct snd_es18xx *chip)
> +static int __devinit snd_es18xx_initialize(struct snd_es18xx *chip,
> + unsigned long mpu_port,
> + unsigned long fm_port)
> {
> int mask = 0;
>
> @@ -1412,15 +1401,15 @@ static int __devinit snd_es18xx_initialize(struct snd_es18xx *chip)
> if (chip->caps & ES18XX_CONTROL) {
> /* Hardware volume IRQ */
> snd_es18xx_config_write(chip, 0x27, chip->irq);
> - if (chip->fm_port > 0 && chip->fm_port != SNDRV_AUTO_PORT) {
> + if (fm_port > 0 && fm_port != SNDRV_AUTO_PORT) {
> /* FM I/O */
> - snd_es18xx_config_write(chip, 0x62, chip->fm_port >> 8);
> - snd_es18xx_config_write(chip, 0x63, chip->fm_port & 0xff);
> + snd_es18xx_config_write(chip, 0x62, fm_port >> 8);
> + snd_es18xx_config_write(chip, 0x63, fm_port & 0xff);
> }
> - if (chip->mpu_port > 0 && chip->mpu_port != SNDRV_AUTO_PORT) {
> + if (mpu_port > 0 && mpu_port != SNDRV_AUTO_PORT) {
> /* MPU-401 I/O */
> - snd_es18xx_config_write(chip, 0x64, chip->mpu_port >> 8);
> - snd_es18xx_config_write(chip, 0x65, chip->mpu_port & 0xff);
> + snd_es18xx_config_write(chip, 0x64, mpu_port >> 8);
> + snd_es18xx_config_write(chip, 0x65, mpu_port & 0xff);
> /* MPU-401 IRQ */
> snd_es18xx_config_write(chip, 0x28, chip->irq);
> }
> @@ -1507,11 +1496,12 @@ static int __devinit snd_es18xx_initialize(struct snd_es18xx *chip)
> snd_es18xx_mixer_write(chip, 0x7A, 0x68);
> /* Enable and set hardware volume interrupt */
> snd_es18xx_mixer_write(chip, 0x64, 0x06);
> - if (chip->mpu_port > 0 && chip->mpu_port != SNDRV_AUTO_PORT) {
> + if (mpu_port > 0 && mpu_port != SNDRV_AUTO_PORT) {
> /* MPU401 share irq with audio
> Joystick enabled
> FM enabled */
> - snd_es18xx_mixer_write(chip, 0x40, 0x43 | (chip->mpu_port & 0xf0) >> 1);
> + snd_es18xx_mixer_write(chip, 0x40,
> + 0x43 | (mpu_port & 0xf0) >> 1);
> }
> snd_es18xx_mixer_write(chip, 0x7f, ((irqmask + 1) << 1) | 0x01);
> }
> @@ -1629,7 +1619,9 @@ static int __devinit snd_es18xx_identify(struct snd_es18xx *chip)
> return 0;
> }
>
> -static int __devinit snd_es18xx_probe(struct snd_es18xx *chip)
> +static int __devinit snd_es18xx_probe(struct snd_es18xx *chip,
> + unsigned long mpu_port,
> + unsigned long fm_port)
> {
> if (snd_es18xx_identify(chip) < 0) {
> snd_printk(KERN_ERR PFX "[0x%lx] ESS chip not found\n", chip->port);
> @@ -1650,8 +1642,6 @@ static int __devinit snd_es18xx_probe(struct snd_es18xx *chip)
> chip->caps = ES18XX_PCM2 | ES18XX_SPATIALIZER | ES18XX_RECMIX | ES18XX_NEW_RATE | ES18XX_AUXB | ES18XX_I2S | ES18XX_CONTROL | ES18XX_HWV;
> break;
> case 0x1887:
> - chip->caps = ES18XX_PCM2 | ES18XX_RECMIX | ES18XX_AUXB | ES18XX_DUPLEX_SAME;
> - break;
> case 0x1888:
> chip->caps = ES18XX_PCM2 | ES18XX_RECMIX | ES18XX_AUXB | ES18XX_DUPLEX_SAME;
> break;
> @@ -1666,7 +1656,7 @@ static int __devinit snd_es18xx_probe(struct snd_es18xx *chip)
> if (chip->dma1 == chip->dma2)
> chip->caps &= ~(ES18XX_PCM2 | ES18XX_DUPLEX_SAME);
>
> - return snd_es18xx_initialize(chip);
> + return snd_es18xx_initialize(chip, mpu_port, fm_port);
> }
>
> static struct snd_pcm_ops snd_es18xx_playback_ops = {
> @@ -1802,10 +1792,7 @@ static int __devinit snd_es18xx_new_device(struct snd_card *card,
>
> spin_lock_init(&chip->reg_lock);
> spin_lock_init(&chip->mixer_lock);
> - spin_lock_init(&chip->ctrl_lock);
> chip->port = port;
> - chip->mpu_port = mpu_port;
> - chip->fm_port = fm_port;
> chip->irq = -1;
> chip->dma1 = -1;
> chip->dma2 = -1;
> @@ -1841,11 +1828,11 @@ static int __devinit snd_es18xx_new_device(struct snd_card *card,
> }
> chip->dma2 = dma2;
>
> - if (snd_es18xx_probe(chip) < 0) {
> + if (snd_es18xx_probe(chip, mpu_port, fm_port) < 0) {
> snd_es18xx_free(card);
> - return -ENODEV;
> - }
> - err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, NULL, &ops);
> + return -ENODEV;
> + }
> + err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
> if (err < 0) {
> snd_es18xx_free(card);
> return err;
> @@ -1980,7 +1967,7 @@ static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-MAX */
> static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* ID for this card */
> static int enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_ISAPNP; /* Enable this card */
> #ifdef CONFIG_PNP
> -static int isapnp[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 1};
> +static int isapnp[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_ISAPNP;
> #endif
> static long port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT; /* 0x220,0x240,0x260,0x280 */
> #ifndef CONFIG_PNP
> @@ -2160,19 +2147,23 @@ static int __devinit snd_audiodrive_probe(struct snd_card *card, int dev)
> return err;
>
> if (fm_port[dev] > 0 && fm_port[dev] != SNDRV_AUTO_PORT) {
> - if (snd_opl3_create(card, chip->fm_port, chip->fm_port + 2, OPL3_HW_OPL3, 0, &opl3) < 0) {
> - snd_printk(KERN_WARNING PFX "opl3 not detected at 0x%lx\n", chip->fm_port);
> + if (snd_opl3_create(card, fm_port[dev], fm_port[dev] + 2,
> + OPL3_HW_OPL3, 0, &opl3) < 0) {
> + snd_printk(KERN_WARNING PFX
> + "opl3 not detected at 0x%lx\n",
> + fm_port[dev]);
> } else {
> - if ((err = snd_opl3_hwdep_new(opl3, 0, 1, NULL)) < 0)
> + err = snd_opl3_hwdep_new(opl3, 0, 1, NULL);
> + if (err < 0)
> return err;
> }
> }
>
> if (mpu_port[dev] > 0 && mpu_port[dev] != SNDRV_AUTO_PORT) {
> - if ((err = snd_mpu401_uart_new(card, 0, MPU401_HW_ES18XX,
> - chip->mpu_port, 0,
> - irq[dev], 0,
> - &chip->rmidi)) < 0)
> + err = snd_mpu401_uart_new(card, 0, MPU401_HW_ES18XX,
> + mpu_port[dev], 0,
> + irq[dev], 0, &chip->rmidi);
> + if (err < 0)
> return err;
> }
>
> --
> 1.6.4
>
>
> Tanie rozmowy telefoniczne!
> Sprawdz >> http://link.interia.pl/f2410
>
More information about the Alsa-devel
mailing list