[PATCH v2 1/2] ALSA: hda: fix the missing ptr initialization

[PATCH v2 1/2] ALSA: hda: fix the missing ptr initialization

[Vinod Koul]

ebus is a member of extended device and was never initialized, so
do this at device creation.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/hda/ext/hdac_ext_bus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
index 2433f7c81472..64de0a3d6d93 100644
--- a/sound/hda/ext/hdac_ext_bus.c
+++ b/sound/hda/ext/hdac_ext_bus.c
@@ -144,6 +144,7 @@ int snd_hdac_ext_bus_device_init(struct hdac_ext_bus *ebus, int addr)
 	if (!edev)
 		return -ENOMEM;
 	hdev = &edev->hdac;
+	edev->ebus = ebus;
 
 	snprintf(name, sizeof(name), "ehdaudio%dD%d", ebus->idx, addr);
 
-- 
1.9.1

[PATCH v2 2/2] ALSA: hda: fix to wait for RIRB & CORB DMA to set

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

If the DMAs are not being quiesced properly, it may lead to
stability issues, so the recommendation is to wait till DMAs are
stopped.

After setting the stop bit of RIRB/CORB DMA, we should wait for
stop bit to be set.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
changes in v2:
 - use common wait routine as suggested by Takashi

 sound/hda/hdac_controller.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index 8c486235c905..9fee464e5d49 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -80,6 +80,22 @@ void snd_hdac_bus_init_cmd_io(struct hdac_bus *bus)
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_init_cmd_io);
 
+/* wait for cmd dmas till they are stopped */
+static void hdac_wait_for_cmd_dmas(struct hdac_bus *bus)
+{
+	unsigned long timeout;
+
+	timeout = jiffies + msecs_to_jiffies(100);
+	while ((snd_hdac_chip_readb(bus, RIRBCTL) & AZX_RBCTL_DMA_EN)
+		&& time_before(jiffies, timeout))
+		udelay(10);
+
+	timeout = jiffies + msecs_to_jiffies(100);
+	while ((snd_hdac_chip_readb(bus, CORBCTL) & AZX_CORBCTL_RUN)
+		&& time_before(jiffies, timeout))
+		udelay(10);
+}
+
 /**
  * snd_hdac_bus_stop_cmd_io - clean up CORB/RIRB buffers
  * @bus: HD-audio core bus
@@ -90,6 +106,7 @@ void snd_hdac_bus_stop_cmd_io(struct hdac_bus *bus)
 	/* disable ringbuffer DMAs */
 	snd_hdac_chip_writeb(bus, RIRBCTL, 0);
 	snd_hdac_chip_writeb(bus, CORBCTL, 0);
+	hdac_wait_for_cmd_dmas(bus);
 	/* disable unsolicited responses */
 	snd_hdac_chip_updatel(bus, GCTL, AZX_GCTL_UNSOL, 0);
 	spin_unlock_irq(&bus->reg_lock);
-- 
1.9.1

Re: [PATCH v2 2/2] ALSA: hda: fix to wait for RIRB & CORB DMA to set

[Takashi Iwai]

On Thu, 05 May 2016 07:54:43 +0200,
Vinod Koul wrote:
> 
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> If the DMAs are not being quiesced properly, it may lead to
> stability issues, so the recommendation is to wait till DMAs are
> stopped.
> 
> After setting the stop bit of RIRB/CORB DMA, we should wait for
> stop bit to be set.
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
> changes in v2:
>  - use common wait routine as suggested by Takashi
> 
>  sound/hda/hdac_controller.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
> index 8c486235c905..9fee464e5d49 100644
> --- a/sound/hda/hdac_controller.c
> +++ b/sound/hda/hdac_controller.c
> @@ -80,6 +80,22 @@ void snd_hdac_bus_init_cmd_io(struct hdac_bus *bus)
>  }
>  EXPORT_SYMBOL_GPL(snd_hdac_bus_init_cmd_io);
>  
> +/* wait for cmd dmas till they are stopped */
> +static void hdac_wait_for_cmd_dmas(struct hdac_bus *bus)
> +{
> +	unsigned long timeout;
> +
> +	timeout = jiffies + msecs_to_jiffies(100);
> +	while ((snd_hdac_chip_readb(bus, RIRBCTL) & AZX_RBCTL_DMA_EN)
> +		&& time_before(jiffies, timeout))
> +		udelay(10);
> +
> +	timeout = jiffies + msecs_to_jiffies(100);
> +	while ((snd_hdac_chip_readb(bus, CORBCTL) & AZX_CORBCTL_RUN)
> +		&& time_before(jiffies, timeout))
> +		udelay(10);

This will end up with doubled timeout.  But it doesn't matter so much
in practice.  So I applied the patch as is.


thanks,

Takashi


> +}
> +
>  /**
>   * snd_hdac_bus_stop_cmd_io - clean up CORB/RIRB buffers
>   * @bus: HD-audio core bus
> @@ -90,6 +106,7 @@ void snd_hdac_bus_stop_cmd_io(struct hdac_bus *bus)
>  	/* disable ringbuffer DMAs */
>  	snd_hdac_chip_writeb(bus, RIRBCTL, 0);
>  	snd_hdac_chip_writeb(bus, CORBCTL, 0);
> +	hdac_wait_for_cmd_dmas(bus);
>  	/* disable unsolicited responses */
>  	snd_hdac_chip_updatel(bus, GCTL, AZX_GCTL_UNSOL, 0);
>  	spin_unlock_irq(&bus->reg_lock);
> -- 
> 1.9.1
> 

Re: [PATCH v2 2/2] ALSA: hda: fix to wait for RIRB & CORB DMA to set

[Vinod Koul]

On Sun, May 08, 2016 at 11:45:50AM +0200, Takashi Iwai wrote:
> > +/* wait for cmd dmas till they are stopped */
> > +static void hdac_wait_for_cmd_dmas(struct hdac_bus *bus)
> > +{
> > +	unsigned long timeout;
> > +
> > +	timeout = jiffies + msecs_to_jiffies(100);
> > +	while ((snd_hdac_chip_readb(bus, RIRBCTL) & AZX_RBCTL_DMA_EN)
> > +		&& time_before(jiffies, timeout))
> > +		udelay(10);
> > +
> > +	timeout = jiffies + msecs_to_jiffies(100);
> > +	while ((snd_hdac_chip_readb(bus, CORBCTL) & AZX_CORBCTL_RUN)
> > +		&& time_before(jiffies, timeout))
> > +		udelay(10);
> 
> This will end up with doubled timeout.  But it doesn't matter so much
> in practice.  So I applied the patch as is.

Thanks Takashi

-- 
~Vinod

Re: [PATCH v2 1/2] ALSA: hda: fix the missing ptr initialization

[Takashi Iwai]

On Thu, 05 May 2016 07:54:42 +0200,
Vinod Koul wrote:
> 
> ebus is a member of extended device and was never initialized, so
> do this at device creation.
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>

Applied, thanks.


Takashi

> ---
>  sound/hda/ext/hdac_ext_bus.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
> index 2433f7c81472..64de0a3d6d93 100644
> --- a/sound/hda/ext/hdac_ext_bus.c
> +++ b/sound/hda/ext/hdac_ext_bus.c
> @@ -144,6 +144,7 @@ int snd_hdac_ext_bus_device_init(struct hdac_ext_bus *ebus, int addr)
>  	if (!edev)
>  		return -ENOMEM;
>  	hdev = &edev->hdac;
> +	edev->ebus = ebus;
>  
>  	snprintf(name, sizeof(name), "ehdaudio%dD%d", ebus->idx, addr);
>  
> -- 
> 1.9.1
>