[alsa-devel] [PATCHv2 0/2] fm801: small clean up series

newer
[alsa-devel] No sound on Thinkpad...

older
[alsa-devel] Invalid PCM pointer...

Andy Shevchenko

29 Apr 2014 29 Apr '14

10:56 a.m.

Two patches in the series are targeting to clean up a bit fm801 driver.

Changes since v1: - remove inline from helper function definitions - fix rebase issue that led to wrong logic - rename helper function to be more particular - reorder parameters in the hw accessor macros

Andy Shevchenko (2): fm801: introduce macros to access the hardware fm801: introduce fm801_ac97_is_ready()/fm801_ac97_is_valid() helpers

sound/pci/fm801.c | 200 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 105 insertions(+), 95 deletions(-)

-- 1.8.3.101.g727a46b

Show replies by date

Andy Shevchenko

29 Apr 29 Apr

10:56 a.m.

New subject: [alsa-devel] [PATCHv2 1/2] fm801: introduce macros to access the hardware

It will help to maintain HW accessors and, for example, switch from the direct I/O to MMIO which is more convenient for PCI devices.

Signed-off-by: Andy Shevchenko andy.shevchenko@gmail.com --- sound/pci/fm801.c | 131 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 68 insertions(+), 63 deletions(-)

diff --git a/sound/pci/fm801.c b/sound/pci/fm801.c index db18cca..5be910c 100644 --- a/sound/pci/fm801.c +++ b/sound/pci/fm801.c @@ -23,6 +23,7 @@ #include <linux/delay.h> #include <linux/init.h> #include <linux/interrupt.h> +#include <linux/io.h> #include <linux/pci.h> #include <linux/slab.h> #include <linux/module.h> @@ -34,8 +35,6 @@ #include <sound/opl3.h> #include <sound/initval.h>

-#include <asm/io.h> - #ifdef CONFIG_SND_FM801_TEA575X_BOOL #include <media/tea575x.h> #endif @@ -80,7 +79,10 @@ MODULE_PARM_DESC(radio_nr, "Radio device numbers"); * Direct registers */

-#define FM801_REG(chip, reg) (chip->port + FM801_##reg) +#define fm801_writew(chip,reg,value) outw((value), chip->port + FM801_##reg) +#define fm801_readw(chip,reg) inw(chip->port + FM801_##reg) + +#define fm801_writel(chip,reg,value) outl((value), chip->port + FM801_##reg)

#define FM801_PCM_VOL 0x00 /* PCM Output Volume */ #define FM801_FM_VOL 0x02 /* FM Output Volume */ @@ -250,7 +252,7 @@ static void snd_fm801_codec_write(struct snd_ac97 *ac97, * Wait until the codec interface is not ready.. */ for (idx = 0; idx < 100; idx++) { - if (!(inw(FM801_REG(chip, AC97_CMD)) & FM801_AC97_BUSY)) + if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) goto ok1; udelay(10); } @@ -259,13 +261,13 @@ static void snd_fm801_codec_write(struct snd_ac97 *ac97,

ok1: /* write data and address */ - outw(val, FM801_REG(chip, AC97_DATA)); - outw(reg | (ac97->addr << FM801_AC97_ADDR_SHIFT), FM801_REG(chip, AC97_CMD)); + fm801_writew(chip, AC97_DATA, val); + fm801_writew(chip, AC97_CMD, reg | (ac97->addr << FM801_AC97_ADDR_SHIFT)); /* * Wait until the write command is not completed.. */ for (idx = 0; idx < 1000; idx++) { - if (!(inw(FM801_REG(chip, AC97_CMD)) & FM801_AC97_BUSY)) + if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) return; udelay(10); } @@ -281,7 +283,7 @@ static unsigned short snd_fm801_codec_read(struct snd_ac97 *ac97, unsigned short * Wait until the codec interface is not ready.. */ for (idx = 0; idx < 100; idx++) { - if (!(inw(FM801_REG(chip, AC97_CMD)) & FM801_AC97_BUSY)) + if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) goto ok1; udelay(10); } @@ -290,10 +292,10 @@ static unsigned short snd_fm801_codec_read(struct snd_ac97 *ac97, unsigned short

ok1: /* read command */ - outw(reg | (ac97->addr << FM801_AC97_ADDR_SHIFT) | FM801_AC97_READ, - FM801_REG(chip, AC97_CMD)); + fm801_writew(chip, AC97_CMD, + reg | (ac97->addr << FM801_AC97_ADDR_SHIFT) | FM801_AC97_READ); for (idx = 0; idx < 100; idx++) { - if (!(inw(FM801_REG(chip, AC97_CMD)) & FM801_AC97_BUSY)) + if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) goto ok2; udelay(10); } @@ -302,7 +304,7 @@ static unsigned short snd_fm801_codec_read(struct snd_ac97 *ac97, unsigned short

ok2: for (idx = 0; idx < 1000; idx++) { - if (inw(FM801_REG(chip, AC97_CMD)) & FM801_AC97_VALID) + if (fm801_readw(chip, AC97_CMD) & FM801_AC97_VALID) goto ok3; udelay(10); } @@ -310,7 +312,7 @@ static unsigned short snd_fm801_codec_read(struct snd_ac97 *ac97, unsigned short return 0;

ok3: - return inw(FM801_REG(chip, AC97_DATA)); + return fm801_readw(chip, AC97_DATA); }

static unsigned int rates[] = { @@ -384,7 +386,7 @@ static int snd_fm801_playback_trigger(struct snd_pcm_substream *substream, snd_BUG(); return -EINVAL; } - outw(chip->ply_ctrl, FM801_REG(chip, PLY_CTRL)); + fm801_writew(chip, PLY_CTRL, chip->ply_ctrl); spin_unlock(&chip->reg_lock); return 0; } @@ -419,7 +421,7 @@ static int snd_fm801_capture_trigger(struct snd_pcm_substream *substream, snd_BUG(); return -EINVAL; } - outw(chip->cap_ctrl, FM801_REG(chip, CAP_CTRL)); + fm801_writew(chip, CAP_CTRL, chip->cap_ctrl); spin_unlock(&chip->reg_lock); return 0; } @@ -457,12 +459,13 @@ static int snd_fm801_playback_prepare(struct snd_pcm_substream *substream) } chip->ply_ctrl |= snd_fm801_rate_bits(runtime->rate) << FM801_RATE_SHIFT; chip->ply_buf = 0; - outw(chip->ply_ctrl, FM801_REG(chip, PLY_CTRL)); - outw(chip->ply_count - 1, FM801_REG(chip, PLY_COUNT)); + fm801_writew(chip, PLY_CTRL, chip->ply_ctrl); + fm801_writew(chip, PLY_COUNT, chip->ply_count - 1); chip->ply_buffer = runtime->dma_addr; chip->ply_pos = 0; - outl(chip->ply_buffer, FM801_REG(chip, PLY_BUF1)); - outl(chip->ply_buffer + (chip->ply_count % chip->ply_size), FM801_REG(chip, PLY_BUF2)); + fm801_writel(chip, PLY_BUF1, chip->ply_buffer); + fm801_writel(chip, PLY_BUF2, + chip->ply_buffer + (chip->ply_count % chip->ply_size)); spin_unlock_irq(&chip->reg_lock); return 0; } @@ -483,12 +486,13 @@ static int snd_fm801_capture_prepare(struct snd_pcm_substream *substream) chip->cap_ctrl |= FM801_STEREO; chip->cap_ctrl |= snd_fm801_rate_bits(runtime->rate) << FM801_RATE_SHIFT; chip->cap_buf = 0; - outw(chip->cap_ctrl, FM801_REG(chip, CAP_CTRL)); - outw(chip->cap_count - 1, FM801_REG(chip, CAP_COUNT)); + fm801_writew(chip, CAP_CTRL, chip->cap_ctrl); + fm801_writew(chip, CAP_COUNT, chip->cap_count - 1); chip->cap_buffer = runtime->dma_addr; chip->cap_pos = 0; - outl(chip->cap_buffer, FM801_REG(chip, CAP_BUF1)); - outl(chip->cap_buffer + (chip->cap_count % chip->cap_size), FM801_REG(chip, CAP_BUF2)); + fm801_writel(chip, CAP_BUF1, chip->cap_buffer); + fm801_writel(chip, CAP_BUF2, + chip->cap_buffer + (chip->cap_count % chip->cap_size)); spin_unlock_irq(&chip->reg_lock); return 0; } @@ -501,8 +505,8 @@ static snd_pcm_uframes_t snd_fm801_playback_pointer(struct snd_pcm_substream *su if (!(chip->ply_ctrl & FM801_START)) return 0; spin_lock(&chip->reg_lock); - ptr = chip->ply_pos + (chip->ply_count - 1) - inw(FM801_REG(chip, PLY_COUNT)); - if (inw(FM801_REG(chip, IRQ_STATUS)) & FM801_IRQ_PLAYBACK) { + ptr = chip->ply_pos + (chip->ply_count - 1) - fm801_readw(chip, PLY_COUNT); + if (fm801_readw(chip, IRQ_STATUS) & FM801_IRQ_PLAYBACK) { ptr += chip->ply_count; ptr %= chip->ply_size; } @@ -518,8 +522,8 @@ static snd_pcm_uframes_t snd_fm801_capture_pointer(struct snd_pcm_substream *sub if (!(chip->cap_ctrl & FM801_START)) return 0; spin_lock(&chip->reg_lock); - ptr = chip->cap_pos + (chip->cap_count - 1) - inw(FM801_REG(chip, CAP_COUNT)); - if (inw(FM801_REG(chip, IRQ_STATUS)) & FM801_IRQ_CAPTURE) { + ptr = chip->cap_pos + (chip->cap_count - 1) - fm801_readw(chip, CAP_COUNT); + if (fm801_readw(chip, IRQ_STATUS) & FM801_IRQ_CAPTURE) { ptr += chip->cap_count; ptr %= chip->cap_size; } @@ -533,12 +537,12 @@ static irqreturn_t snd_fm801_interrupt(int irq, void *dev_id) unsigned short status; unsigned int tmp;

- status = inw(FM801_REG(chip, IRQ_STATUS)); + status = fm801_readw(chip, IRQ_STATUS); status &= FM801_IRQ_PLAYBACK|FM801_IRQ_CAPTURE|FM801_IRQ_MPU|FM801_IRQ_VOLUME; if (! status) return IRQ_NONE; /* ack first */ - outw(status, FM801_REG(chip, IRQ_STATUS)); + fm801_writew(chip, IRQ_STATUS, status); if (chip->pcm && (status & FM801_IRQ_PLAYBACK) && chip->playback_substream) { spin_lock(&chip->reg_lock); chip->ply_buf++; @@ -546,10 +550,10 @@ static irqreturn_t snd_fm801_interrupt(int irq, void *dev_id) chip->ply_pos %= chip->ply_size; tmp = chip->ply_pos + chip->ply_count; tmp %= chip->ply_size; - outl(chip->ply_buffer + tmp, - (chip->ply_buf & 1) ? - FM801_REG(chip, PLY_BUF1) : - FM801_REG(chip, PLY_BUF2)); + if (chip->ply_buf & 1) + fm801_writel(chip, PLY_BUF1, chip->ply_buffer + tmp); + else + fm801_writel(chip, PLY_BUF2, chip->ply_buffer + tmp); spin_unlock(&chip->reg_lock); snd_pcm_period_elapsed(chip->playback_substream); } @@ -560,10 +564,10 @@ static irqreturn_t snd_fm801_interrupt(int irq, void *dev_id) chip->cap_pos %= chip->cap_size; tmp = chip->cap_pos + chip->cap_count; tmp %= chip->cap_size; - outl(chip->cap_buffer + tmp, - (chip->cap_buf & 1) ? - FM801_REG(chip, CAP_BUF1) : - FM801_REG(chip, CAP_BUF2)); + if (chip->cap_buf & 1) + fm801_writel(chip, CAP_BUF1, chip->cap_buffer + tmp); + else + fm801_writel(chip, CAP_BUF2, chip->cap_buffer + tmp); spin_unlock(&chip->reg_lock); snd_pcm_period_elapsed(chip->capture_substream); } @@ -747,7 +751,7 @@ static struct snd_fm801_tea575x_gpio snd_fm801_tea575x_gpios[] = { static void snd_fm801_tea575x_set_pins(struct snd_tea575x *tea, u8 pins) { struct fm801 *chip = tea->private_data; - unsigned short reg = inw(FM801_REG(chip, GPIO_CTRL)); + unsigned short reg = fm801_readw(chip, GPIO_CTRL); struct snd_fm801_tea575x_gpio gpio = *get_tea575x_gpio(chip);

reg &= ~(FM801_GPIO_GP(gpio.data) | @@ -759,13 +763,13 @@ static void snd_fm801_tea575x_set_pins(struct snd_tea575x *tea, u8 pins) /* WRITE_ENABLE is inverted */ reg |= (pins & TEA575X_WREN) ? 0 : FM801_GPIO_GP(gpio.wren);

- outw(reg, FM801_REG(chip, GPIO_CTRL)); + fm801_writew(chip, GPIO_CTRL, reg); }

static u8 snd_fm801_tea575x_get_pins(struct snd_tea575x *tea) { struct fm801 *chip = tea->private_data; - unsigned short reg = inw(FM801_REG(chip, GPIO_CTRL)); + unsigned short reg = fm801_readw(chip, GPIO_CTRL); struct snd_fm801_tea575x_gpio gpio = *get_tea575x_gpio(chip); u8 ret;

@@ -780,7 +784,7 @@ static u8 snd_fm801_tea575x_get_pins(struct snd_tea575x *tea) static void snd_fm801_tea575x_set_direction(struct snd_tea575x *tea, bool output) { struct fm801 *chip = tea->private_data; - unsigned short reg = inw(FM801_REG(chip, GPIO_CTRL)); + unsigned short reg = fm801_readw(chip, GPIO_CTRL); struct snd_fm801_tea575x_gpio gpio = *get_tea575x_gpio(chip);

/* use GPIO lines and set write enable bit */ @@ -811,7 +815,7 @@ static void snd_fm801_tea575x_set_direction(struct snd_tea575x *tea, bool output FM801_GPIO_GP(gpio.clk)); }

- outw(reg, FM801_REG(chip, GPIO_CTRL)); + fm801_writew(chip, GPIO_CTRL, reg); }

static struct snd_tea575x_ops snd_fm801_tea_ops = { @@ -962,7 +966,7 @@ static int snd_fm801_get_mux(struct snd_kcontrol *kcontrol, struct fm801 *chip = snd_kcontrol_chip(kcontrol); unsigned short val;

- val = inw(FM801_REG(chip, REC_SRC)) & 7; + val = fm801_readw(chip, REC_SRC) & 7; if (val > 4) val = 4; ucontrol->value.enumerated.item[0] = val; @@ -1073,12 +1077,12 @@ static int wait_for_codec(struct fm801 *chip, unsigned int codec_id, { unsigned long timeout = jiffies + waits;

- outw(FM801_AC97_READ | (codec_id << FM801_AC97_ADDR_SHIFT) | reg, - FM801_REG(chip, AC97_CMD)); + fm801_writew(chip, AC97_CMD, + reg | (codec_id << FM801_AC97_ADDR_SHIFT) | FM801_AC97_READ); udelay(5); do { - if ((inw(FM801_REG(chip, AC97_CMD)) & (FM801_AC97_VALID|FM801_AC97_BUSY)) - == FM801_AC97_VALID) + if ((fm801_readw(chip, AC97_CMD) & + (FM801_AC97_VALID | FM801_AC97_BUSY)) == FM801_AC97_VALID) return 0; schedule_timeout_uninterruptible(1); } while (time_after(timeout, jiffies)); @@ -1093,10 +1097,10 @@ static int snd_fm801_chip_init(struct fm801 *chip, int resume) goto __ac97_ok;

/* codec cold reset + AC'97 warm reset */ - outw((1<<5) | (1<<6), FM801_REG(chip, CODEC_CTRL)); - inw(FM801_REG(chip, CODEC_CTRL)); /* flush posting data */ + fm801_writew(chip, CODEC_CTRL, (1 << 5) | (1 << 6)); + fm801_readw(chip, CODEC_CTRL); /* flush posting data */ udelay(100); - outw(0, FM801_REG(chip, CODEC_CTRL)); + fm801_writew(chip, CODEC_CTRL, 0);

if (wait_for_codec(chip, 0, AC97_RESET, msecs_to_jiffies(750)) < 0) if (!resume) { @@ -1117,7 +1121,7 @@ static int snd_fm801_chip_init(struct fm801 *chip, int resume) for (i = 3; i > 0; i--) { if (!wait_for_codec(chip, i, AC97_VENDOR_ID1, msecs_to_jiffies(50))) { - cmdw = inw(FM801_REG(chip, AC97_DATA)); + cmdw = fm801_readw(chip, AC97_DATA); if (cmdw != 0xffff && cmdw != 0) { chip->secondary = 1; chip->secondary_addr = i; @@ -1135,23 +1139,24 @@ static int snd_fm801_chip_init(struct fm801 *chip, int resume) __ac97_ok:

/* init volume */ - outw(0x0808, FM801_REG(chip, PCM_VOL)); - outw(0x9f1f, FM801_REG(chip, FM_VOL)); - outw(0x8808, FM801_REG(chip, I2S_VOL)); + fm801_writew(chip, PCM_VOL, 0x0808); + fm801_writew(chip, FM_VOL, 0x9f1f); + fm801_writew(chip, I2S_VOL, 0x8808);

/* I2S control - I2S mode */ - outw(0x0003, FM801_REG(chip, I2S_MODE)); + fm801_writew(chip, I2S_MODE, 0x0003);

/* interrupt setup */ - cmdw = inw(FM801_REG(chip, IRQ_MASK)); + cmdw = fm801_readw(chip, IRQ_MASK); if (chip->irq < 0) cmdw |= 0x00c3; /* mask everything, no PCM nor MPU */ else cmdw &= ~0x0083; /* unmask MPU, PLAYBACK & CAPTURE */ - outw(cmdw, FM801_REG(chip, IRQ_MASK)); + fm801_writew(chip, IRQ_MASK, cmdw);

/* interrupt clear */ - outw(FM801_IRQ_PLAYBACK|FM801_IRQ_CAPTURE|FM801_IRQ_MPU, FM801_REG(chip, IRQ_STATUS)); + fm801_writew(chip, IRQ_STATUS, + FM801_IRQ_PLAYBACK | FM801_IRQ_CAPTURE | FM801_IRQ_MPU);

return 0; } @@ -1165,9 +1170,9 @@ static int snd_fm801_free(struct fm801 *chip) goto __end_hw;

/* interrupt setup - mask everything */ - cmdw = inw(FM801_REG(chip, IRQ_MASK)); + cmdw = fm801_readw(chip, IRQ_MASK); cmdw |= 0x00c3; - outw(cmdw, FM801_REG(chip, IRQ_MASK)); + fm801_writew(chip, IRQ_MASK, cmdw);

__end_hw: #ifdef CONFIG_SND_FM801_TEA575X_BOOL @@ -1339,15 +1344,15 @@ static int snd_card_fm801_probe(struct pci_dev *pci, return err; } if ((err = snd_mpu401_uart_new(card, 0, MPU401_HW_FM801, - FM801_REG(chip, MPU401_DATA), + chip->port + FM801_MPU401_DATA, MPU401_INFO_INTEGRATED | MPU401_INFO_IRQ_HOOK, -1, &chip->rmidi)) < 0) { snd_card_free(card); return err; } - if ((err = snd_opl3_create(card, FM801_REG(chip, OPL3_BANK0), - FM801_REG(chip, OPL3_BANK1), + if ((err = snd_opl3_create(card, chip->port + FM801_OPL3_BANK0, + chip->port + FM801_OPL3_BANK1, OPL3_HW_OPL3_FM801, 1, &opl3)) < 0) { snd_card_free(card); return err;

-- 1.8.3.101.g727a46b

Andy Shevchenko

10:56 a.m.

New subject: [alsa-devel] [PATCHv2 2/2] fm801: introduce fm801_ac97_is_ready()/fm801_ac97_is_valid() helpers

The introduced functios check AC97 if it's ready for communication and read data is valid.

Signed-off-by: Andy Shevchenko andy.shevchenko@gmail.com --- sound/pci/fm801.c | 79 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 42 insertions(+), 37 deletions(-)

diff --git a/sound/pci/fm801.c b/sound/pci/fm801.c index 5be910c..e8910d0 100644 --- a/sound/pci/fm801.c +++ b/sound/pci/fm801.c @@ -224,6 +224,30 @@ MODULE_DEVICE_TABLE(pci, snd_fm801_ids); * common I/O routines */

+static bool fm801_ac97_is_ready(struct fm801 *chip, unsigned int iterations) +{ + unsigned int idx; + + for (idx = 0; idx < iterations; idx++) { + if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) + return true; + udelay(10); + } + return false; +} + +static bool fm801_ac97_is_valid(struct fm801 *chip, unsigned int iterations) +{ + unsigned int idx; + + for (idx = 0; idx < iterations; idx++) { + if (fm801_readw(chip, AC97_CMD) & FM801_AC97_VALID) + return true; + udelay(10); + } + return false; +} + static int snd_fm801_update_bits(struct fm801 *chip, unsigned short reg, unsigned short mask, unsigned short value) { @@ -246,72 +270,53 @@ static void snd_fm801_codec_write(struct snd_ac97 *ac97, unsigned short val) { struct fm801 *chip = ac97->private_data; - int idx;

/* * Wait until the codec interface is not ready.. */ - for (idx = 0; idx < 100; idx++) { - if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) - goto ok1; - udelay(10); + if (!fm801_ac97_is_ready(chip, 100)) { + dev_err(chip->card->dev, "AC'97 interface is busy (1)\n"); + return; } - dev_err(chip->card->dev, "AC'97 interface is busy (1)\n"); - return;

- ok1: /* write data and address */ fm801_writew(chip, AC97_DATA, val); fm801_writew(chip, AC97_CMD, reg | (ac97->addr << FM801_AC97_ADDR_SHIFT)); /* * Wait until the write command is not completed.. - */ - for (idx = 0; idx < 1000; idx++) { - if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) - return; - udelay(10); - } - dev_err(chip->card->dev, "AC'97 interface #%d is busy (2)\n", ac97->num); + */ + if (!fm801_ac97_is_ready(chip, 1000)) + dev_err(chip->card->dev, "AC'97 interface #%d is busy (2)\n", + ac97->num); }

static unsigned short snd_fm801_codec_read(struct snd_ac97 *ac97, unsigned short reg) { struct fm801 *chip = ac97->private_data; - int idx;

/* * Wait until the codec interface is not ready.. */ - for (idx = 0; idx < 100; idx++) { - if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) - goto ok1; - udelay(10); + if (!fm801_ac97_is_ready(chip, 100)) { + dev_err(chip->card->dev, "AC'97 interface is busy (1)\n"); + return 0; } - dev_err(chip->card->dev, "AC'97 interface is busy (1)\n"); - return 0;

- ok1: /* read command */ fm801_writew(chip, AC97_CMD, reg | (ac97->addr << FM801_AC97_ADDR_SHIFT) | FM801_AC97_READ); - for (idx = 0; idx < 100; idx++) { - if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) - goto ok2; - udelay(10); + if (!fm801_ac97_is_ready(chip, 100)) { + dev_err(chip->card->dev, "AC'97 interface #%d is busy (2)\n", + ac97->num); + return 0; } - dev_err(chip->card->dev, "AC'97 interface #%d is busy (2)\n", ac97->num); - return 0;

- ok2: - for (idx = 0; idx < 1000; idx++) { - if (fm801_readw(chip, AC97_CMD) & FM801_AC97_VALID) - goto ok3; - udelay(10); + if (!fm801_ac97_is_valid(chip, 1000)) { + dev_err(chip->card->dev, + "AC'97 interface #%d is not valid (2)\n", ac97->num); + return 0; } - dev_err(chip->card->dev, "AC'97 interface #%d is not valid (2)\n", ac97->num); - return 0;

- ok3: return fm801_readw(chip, AC97_DATA); }

-- 1.8.3.101.g727a46b

Andy Shevchenko

10:56 a.m.

New subject: [alsa-devel] [PATCH 2/2] fm801: introduce __is_ready()/__is_valid() helpers

The introduced functios check AC97 if it's ready for communication and read data is valid.

Signed-off-by: Andy Shevchenko andy.shevchenko@gmail.com --- sound/pci/fm801.c | 86 +++++++++++++++++++++++++++---------------------------- 1 file changed, 42 insertions(+), 44 deletions(-)

diff --git a/sound/pci/fm801.c b/sound/pci/fm801.c index 8418484..4417409 100644 --- a/sound/pci/fm801.c +++ b/sound/pci/fm801.c @@ -224,6 +224,30 @@ MODULE_DEVICE_TABLE(pci, snd_fm801_ids); * common I/O routines */

+static inline bool __is_ready(struct fm801 *chip, unsigned int iterations) +{ + unsigned int idx; + + for (idx = 0; idx < iterations; idx++) { + if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) + return true; + udelay(10); + } + return false; +} + +static inline bool __is_valid(struct fm801 *chip, unsigned int iterations) +{ + unsigned int idx; + + for (idx = 0; idx < iterations; idx++) { + if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_VALID)) + return true; + udelay(10); + } + return false; +} + static int snd_fm801_update_bits(struct fm801 *chip, unsigned short reg, unsigned short mask, unsigned short value) { @@ -246,32 +270,19 @@ static void snd_fm801_codec_write(struct snd_ac97 *ac97, unsigned short val) { struct fm801 *chip = ac97->private_data; - int idx;

- /* - * Wait until the codec interface is not ready.. - */ - for (idx = 0; idx < 100; idx++) { - if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) - goto ok1; - udelay(10); + if (!__is_ready(chip, 100)) { + dev_err(chip->card->dev, "AC'97 interface is busy (1)\n"); + return; } - dev_err(chip->card->dev, "AC'97 interface is busy (1)\n"); - return;

- ok1: /* write data and address */ fm801_writew(val, chip, AC97_DATA); fm801_writew(reg | (ac97->addr << FM801_AC97_ADDR_SHIFT), chip, AC97_CMD); - /* - * Wait until the write command is not completed.. - */ - for (idx = 0; idx < 1000; idx++) { - if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) - return; - udelay(10); - } - dev_err(chip->card->dev, "AC'97 interface #%d is busy (2)\n", ac97->num); + + if (!__is_ready(chip, 1000)) + dev_err(chip->card->dev, "AC'97 interface #%d is busy (2)\n", + ac97->num); }

static unsigned short snd_fm801_codec_read(struct snd_ac97 *ac97, unsigned short reg) @@ -279,39 +290,26 @@ static unsigned short snd_fm801_codec_read(struct snd_ac97 *ac97, unsigned short struct fm801 *chip = ac97->private_data; int idx;

- /* - * Wait until the codec interface is not ready.. - */ - for (idx = 0; idx < 100; idx++) { - if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) - goto ok1; - udelay(10); + if (!__is_ready(chip, 100)) { + dev_err(chip->card->dev, "AC'97 interface is busy (1)\n"); + return 0; } - dev_err(chip->card->dev, "AC'97 interface is busy (1)\n"); - return 0;

- ok1: /* read command */ fm801_writew(reg | (ac97->addr << FM801_AC97_ADDR_SHIFT) | FM801_AC97_READ, chip, AC97_CMD); - for (idx = 0; idx < 100; idx++) { - if (!(fm801_readw(chip, AC97_CMD) & FM801_AC97_BUSY)) - goto ok2; - udelay(10); + if (!__is_ready(chip, 100)) { + dev_err(chip->card->dev, "AC'97 interface #%d is busy (2)\n", + ac97->num); + return 0; } - dev_err(chip->card->dev, "AC'97 interface #%d is busy (2)\n", ac97->num); - return 0;

- ok2: - for (idx = 0; idx < 1000; idx++) { - if (fm801_readw(chip, AC97_CMD) & FM801_AC97_VALID) - goto ok3; - udelay(10); + if (!__is_valid(chip, 1000)) { + dev_err(chip->card->dev, + "AC'97 interface #%d is not valid (2)\n", ac97->num); + return 0; } - dev_err(chip->card->dev, "AC'97 interface #%d is not valid (2)\n", ac97->num); - return 0;

- ok3: return fm801_readw(chip, AC97_DATA); }

-- 1.8.3.101.g727a46b

Takashi Iwai

5:31 p.m.

At Tue, 29 Apr 2014 10:56:02 +0300, Andy Shevchenko wrote:

...

Two patches in the series are targeting to clean up a bit fm801 driver.

Changes since v1:

remove inline from helper function definitions

fix rebase issue that led to wrong logic

rename helper function to be more particular

reorder parameters in the hw accessor macros

Andy Shevchenko (2): fm801: introduce macros to access the hardware fm801: introduce fm801_ac97_is_ready()/fm801_ac97_is_valid() helpers

Thanks, applied both patches now.

Takashi

...

sound/pci/fm801.c | 200 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 105 insertions(+), 95 deletions(-)

-- 1.8.3.101.g727a46b

3851

Age (days ago)

3851

Last active (days ago)

List overview

Download

4 comments

2 participants

tags (0)

participants (2)

Andy Shevchenko
Takashi Iwai