[alsa-devel] [PATCH 1/2] ALSA: hda: Fix mismatches for register mask and value in hdac controller
E.g. for azx_int_enable(), we should set both mask and value to be "AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN"(the mask was 0) to enable controller CIE and GIE.
We have similar issues on setting AZX_GCTL_RESET and AZX_GCTL_UNSOL, here try to correct all of them.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com --- sound/hda/hdac_controller.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c index 74244d8e2909..b2e9454f5816 100644 --- a/sound/hda/hdac_controller.c +++ b/sound/hda/hdac_controller.c @@ -376,7 +376,7 @@ void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus) { unsigned long timeout;
- snd_hdac_chip_updateb(bus, GCTL, 0, AZX_GCTL_RESET); + snd_hdac_chip_updateb(bus, GCTL, AZX_GCTL_RESET, AZX_GCTL_RESET);
timeout = jiffies + msecs_to_jiffies(100); while (!snd_hdac_chip_readb(bus, GCTL) && time_before(jiffies, timeout)) @@ -415,7 +415,7 @@ int snd_hdac_bus_reset_link(struct hdac_bus *bus, bool full_reset) }
/* Accept unsolicited responses */ - snd_hdac_chip_updatel(bus, GCTL, 0, AZX_GCTL_UNSOL); + snd_hdac_chip_updatel(bus, GCTL, AZX_GCTL_UNSOL, AZX_GCTL_UNSOL);
/* detect codecs */ if (!bus->codec_mask) { @@ -431,7 +431,9 @@ EXPORT_SYMBOL_GPL(snd_hdac_bus_reset_link); static void azx_int_enable(struct hdac_bus *bus) { /* enable controller CIE and GIE */ - snd_hdac_chip_updatel(bus, INTCTL, 0, AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN); + snd_hdac_chip_updatel(bus, INTCTL, + AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN, + AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN); }
/* disable interrupts */
To enable SIE(Stream Interrupt Enable) in snd_hdac_stream_start(), we should set both mask and value to be "1 << azx_dev->index" for register update, the mask was 0, here fix it.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com --- sound/hda/hdac_stream.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c index eee422390d8e..ba73a33480b6 100644 --- a/sound/hda/hdac_stream.c +++ b/sound/hda/hdac_stream.c @@ -56,7 +56,9 @@ void snd_hdac_stream_start(struct hdac_stream *azx_dev, bool fresh_start) azx_dev->start_wallclk -= azx_dev->period_wallclk;
/* enable SIE */ - snd_hdac_chip_updatel(bus, INTCTL, 0, 1 << azx_dev->index); + snd_hdac_chip_updatel(bus, INTCTL, + 1 << azx_dev->index, + 1 << azx_dev->index); /* set DMA start and interrupt mask */ snd_hdac_stream_updateb(azx_dev, SD_CTL, 0, SD_CTL_DMA_START | SD_INT_MASK);
On Wed, 09 Jan 2019 09:20:50 +0100, Keyon Jie wrote:
E.g. for azx_int_enable(), we should set both mask and value to be "AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN"(the mask was 0) to enable controller CIE and GIE.
We have similar issues on setting AZX_GCTL_RESET and AZX_GCTL_UNSOL, here try to correct all of them.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com
Well, the current code isn't incorrect, it serves for its purpose -- set the given bits. But yeah it's clearer to pass the mask bits explicitly.
So I queued these two patches to for-next branch, for 5.1 kernel.
thanks,
Takashi
sound/hda/hdac_controller.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c index 74244d8e2909..b2e9454f5816 100644 --- a/sound/hda/hdac_controller.c +++ b/sound/hda/hdac_controller.c @@ -376,7 +376,7 @@ void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus) { unsigned long timeout;
- snd_hdac_chip_updateb(bus, GCTL, 0, AZX_GCTL_RESET);
snd_hdac_chip_updateb(bus, GCTL, AZX_GCTL_RESET, AZX_GCTL_RESET);
timeout = jiffies + msecs_to_jiffies(100); while (!snd_hdac_chip_readb(bus, GCTL) && time_before(jiffies, timeout))
@@ -415,7 +415,7 @@ int snd_hdac_bus_reset_link(struct hdac_bus *bus, bool full_reset) }
/* Accept unsolicited responses */
- snd_hdac_chip_updatel(bus, GCTL, 0, AZX_GCTL_UNSOL);
snd_hdac_chip_updatel(bus, GCTL, AZX_GCTL_UNSOL, AZX_GCTL_UNSOL);
/* detect codecs */ if (!bus->codec_mask) {
@@ -431,7 +431,9 @@ EXPORT_SYMBOL_GPL(snd_hdac_bus_reset_link); static void azx_int_enable(struct hdac_bus *bus) { /* enable controller CIE and GIE */
- snd_hdac_chip_updatel(bus, INTCTL, 0, AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN);
- snd_hdac_chip_updatel(bus, INTCTL,
AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN,AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN);}
/* disable interrupts */
2.17.1
On 2019/1/9 下午5:31, Takashi Iwai wrote:
On Wed, 09 Jan 2019 09:20:50 +0100, Keyon Jie wrote:
E.g. for azx_int_enable(), we should set both mask and value to be "AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN"(the mask was 0) to enable controller CIE and GIE.
We have similar issues on setting AZX_GCTL_RESET and AZX_GCTL_UNSOL, here try to correct all of them.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com
Well, the current code isn't incorrect, it serves for its purpose -- set the given bits. But yeah it's clearer to pass the mask bits
Yes, I realized that after patches sent out, and that explains why there is no failure ever reported about it.
Thanks, ~Keyon
explicitly.
So I queued these two patches to for-next branch, for 5.1 kernel.
thanks,
Takashi
sound/hda/hdac_controller.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c index 74244d8e2909..b2e9454f5816 100644 --- a/sound/hda/hdac_controller.c +++ b/sound/hda/hdac_controller.c @@ -376,7 +376,7 @@ void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus) { unsigned long timeout;
- snd_hdac_chip_updateb(bus, GCTL, 0, AZX_GCTL_RESET);
snd_hdac_chip_updateb(bus, GCTL, AZX_GCTL_RESET, AZX_GCTL_RESET);
timeout = jiffies + msecs_to_jiffies(100); while (!snd_hdac_chip_readb(bus, GCTL) && time_before(jiffies, timeout))
@@ -415,7 +415,7 @@ int snd_hdac_bus_reset_link(struct hdac_bus *bus, bool full_reset) }
/* Accept unsolicited responses */
- snd_hdac_chip_updatel(bus, GCTL, 0, AZX_GCTL_UNSOL);
snd_hdac_chip_updatel(bus, GCTL, AZX_GCTL_UNSOL, AZX_GCTL_UNSOL);
/* detect codecs */ if (!bus->codec_mask) {
@@ -431,7 +431,9 @@ EXPORT_SYMBOL_GPL(snd_hdac_bus_reset_link); static void azx_int_enable(struct hdac_bus *bus) { /* enable controller CIE and GIE */
- snd_hdac_chip_updatel(bus, INTCTL, 0, AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN);
snd_hdac_chip_updatel(bus, INTCTL,
AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN,AZX_INT_CTRL_EN | AZX_INT_GLOBAL_EN);}
/* disable interrupts */
-- 2.17.1
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (2)
-
Keyon Jie -
Takashi Iwai