* [PATCH 1/2] fm801: introduce macros to access the hardware
@ 2014-04-28 8:00 Andy Shevchenko
2014-04-28 8:00 ` [PATCH 2/2] fm801: introduce __is_ready()/__is_valid() helpers Andy Shevchenko
2014-04-28 10:20 ` [PATCH 1/2] fm801: introduce macros to access the hardware Takashi Iwai
0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2014-04-28 8:00 UTC (permalink / raw)
To: Takashi Iwai, alsa-devel; +Cc: Andy Shevchenko
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..8418484 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(v,chip,reg) outw((v), chip->port + FM801_##reg)
+#define fm801_readw(chip,reg) inw(chip->port + FM801_##reg)
+
+#define fm801_writel(v,chip,reg) outl((v), 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(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 (!(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(reg | (ac97->addr << FM801_AC97_ADDR_SHIFT) |
+ FM801_AC97_READ, chip, AC97_CMD);
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 - 1, chip, PLY_COUNT);
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_buffer, chip, PLY_BUF1);
+ fm801_writel(chip->ply_buffer + (chip->ply_count % chip->ply_size),
+ chip, PLY_BUF2);
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 - 1, chip, CAP_COUNT);
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_buffer, chip, CAP_BUF1);
+ fm801_writel(chip->cap_buffer + (chip->cap_count % chip->cap_size),
+ chip, CAP_BUF2);
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(status, chip, IRQ_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_buffer + tmp, chip, PLY_BUF1);
+ else
+ fm801_writel(chip->ply_buffer + tmp, chip, PLY_BUF2);
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_buffer + tmp, chip, CAP_BUF1);
+ else
+ fm801_writel(chip->cap_buffer + tmp, chip, CAP_BUF2);
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(reg, chip, GPIO_CTRL);
}
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(reg, chip, GPIO_CTRL);
}
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(reg | (codec_id << FM801_AC97_ADDR_SHIFT) |
+ FM801_AC97_READ, chip, AC97_CMD);
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((1 << 5) | (1 << 6), chip, CODEC_CTRL);
+ fm801_readw(chip, CODEC_CTRL); /* flush posting data */
udelay(100);
- outw(0, FM801_REG(chip, CODEC_CTRL));
+ fm801_writew(0, chip, CODEC_CTRL);
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(0x0808, chip, PCM_VOL);
+ fm801_writew(0x9f1f, chip, FM_VOL);
+ fm801_writew(0x8808, chip, I2S_VOL);
/* I2S control - I2S mode */
- outw(0x0003, FM801_REG(chip, I2S_MODE));
+ fm801_writew(0x0003, chip, I2S_MODE);
/* 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(cmdw, chip, IRQ_MASK);
/* interrupt clear */
- outw(FM801_IRQ_PLAYBACK|FM801_IRQ_CAPTURE|FM801_IRQ_MPU, FM801_REG(chip, IRQ_STATUS));
+ fm801_writew(FM801_IRQ_PLAYBACK | FM801_IRQ_CAPTURE | FM801_IRQ_MPU,
+ chip, IRQ_STATUS);
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(cmdw, chip, IRQ_MASK);
__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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] fm801: introduce __is_ready()/__is_valid() helpers
2014-04-28 8:00 [PATCH 1/2] fm801: introduce macros to access the hardware Andy Shevchenko
@ 2014-04-28 8:00 ` Andy Shevchenko
2014-04-28 10:25 ` Takashi Iwai
2014-04-28 10:20 ` [PATCH 1/2] fm801: introduce macros to access the hardware Takashi Iwai
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2014-04-28 8:00 UTC (permalink / raw)
To: Takashi Iwai, alsa-devel; +Cc: Andy Shevchenko
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] fm801: introduce macros to access the hardware
2014-04-28 8:00 [PATCH 1/2] fm801: introduce macros to access the hardware Andy Shevchenko
2014-04-28 8:00 ` [PATCH 2/2] fm801: introduce __is_ready()/__is_valid() helpers Andy Shevchenko
@ 2014-04-28 10:20 ` Takashi Iwai
2014-04-28 11:21 ` Andy Shevchenko
1 sibling, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2014-04-28 10:20 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: alsa-devel
At Mon, 28 Apr 2014 11:00:29 +0300,
Andy Shevchenko wrote:
>
> 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..8418484 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(v,chip,reg) outw((v), chip->port + FM801_##reg)
> +#define fm801_readw(chip,reg) inw(chip->port + FM801_##reg)
> +
> +#define fm801_writel(v,chip,reg) outl((v), chip->port + FM801_##reg)
IMO, it's better in a form like fm801_writel(chip, reg, value)
thanks,
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] fm801: introduce __is_ready()/__is_valid() helpers
2014-04-28 8:00 ` [PATCH 2/2] fm801: introduce __is_ready()/__is_valid() helpers Andy Shevchenko
@ 2014-04-28 10:25 ` Takashi Iwai
2014-04-28 11:26 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2014-04-28 10:25 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: alsa-devel
At Mon, 28 Apr 2014 11:00:30 +0300,
Andy Shevchenko wrote:
>
> 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;
> +}
The function name is a bit too ambiguous, and you don't have to use
"__" prefix unnecessarily. Also, don't add inline unless really
needed. Compilers are often smarter than us.
> +
> +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;
> +}
Does the patch really work as expected? It returns true when no VALID
bit is set.
thanks,
Takashi
> +
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] fm801: introduce macros to access the hardware
2014-04-28 10:20 ` [PATCH 1/2] fm801: introduce macros to access the hardware Takashi Iwai
@ 2014-04-28 11:21 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2014-04-28 11:21 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On Mon, Apr 28, 2014 at 1:20 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Mon, 28 Apr 2014 11:00:29 +0300,
> Andy Shevchenko wrote:
> >
> > 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..8418484 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(v,chip,reg) outw((v), chip->port + FM801_##reg)
> > +#define fm801_readw(chip,reg) inw(chip->port +
> FM801_##reg)
> > +
> > +#define fm801_writel(v,chip,reg) outl((v), chip->port + FM801_##reg)
>
> IMO, it's better in a form like fm801_writel(chip, reg, value)
Will change this in v2.
> thanks,
>
> Takashi
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] fm801: introduce __is_ready()/__is_valid() helpers
2014-04-28 10:25 ` Takashi Iwai
@ 2014-04-28 11:26 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2014-04-28 11:26 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On Mon, Apr 28, 2014 at 1:25 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Mon, 28 Apr 2014 11:00:30 +0300,
> Andy Shevchenko wrote:
> >
> > 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;
> > +}
>
> The function name is a bit too ambiguous, and you don't have to use
> "__" prefix unnecessarily. Also, don't add inline unless really
> needed. Compilers are often smarter than us.
>
Understood. Will change this in v2.
>
> > +
> > +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;
> > +}
>
> Does the patch really work as expected? It returns true when no VALID
> bit is set.
>
Oops, rebase issue. Thanks for caught this up!
>
>
> thanks,
>
> Takashi
>
>
> > +
> > 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
> >
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-28 11:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-28 8:00 [PATCH 1/2] fm801: introduce macros to access the hardware Andy Shevchenko
2014-04-28 8:00 ` [PATCH 2/2] fm801: introduce __is_ready()/__is_valid() helpers Andy Shevchenko
2014-04-28 10:25 ` Takashi Iwai
2014-04-28 11:26 ` Andy Shevchenko
2014-04-28 10:20 ` [PATCH 1/2] fm801: introduce macros to access the hardware Takashi Iwai
2014-04-28 11:21 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.