[PATCH 0/4] ALSA: hda: Minor cleanups

[PATCH 0/4] ALSA: hda: Minor cleanups

[Amadeusz Sławiński]

First two patches are small refactoring of code to use poll macros
instead of open coding register checks.
Following two patches remove unused defines from code.

Amadeusz Sławiński (4):
  ALSA: hda: Move stream-register polling macros
  ALSA: hda: Rework snd_hdac_stream_reset() to use macros
  ALSA: hda: Remove unused MAX_PIN_CONFIGS constant
  ALSA: hda: Remove unused defines

 include/sound/hdaudio.h     |  6 ++++++
 include/sound/hdaudio_ext.h |  6 ------
 sound/hda/hdac_stream.c     | 27 +++++++--------------------
 sound/pci/hda/hda_intel.c   |  7 -------
 sound/pci/hda/hda_sysfs.c   |  2 --
 5 files changed, 13 insertions(+), 35 deletions(-)

-- 
2.25.1

[PATCH 1/4] ALSA: hda: Move stream-register polling macros

[Amadeusz Sławiński]

Polling stream registers doesn't really have anything to do with
extended HDA registers, so move it to basic HDA header. This will allow
for use in HDA framework.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 include/sound/hdaudio.h     | 6 ++++++
 include/sound/hdaudio_ext.h | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 797bf67a164d..a46708b2d8ea 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -589,6 +589,12 @@ int snd_hdac_get_stream_stripe_ctl(struct hdac_bus *bus,
 	snd_hdac_reg_readw((dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg)
 #define snd_hdac_stream_readb(dev, reg) \
 	snd_hdac_reg_readb((dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg)
+#define snd_hdac_stream_readb_poll(dev, reg, val, cond, delay_us, timeout_us) \
+	readb_poll_timeout((dev)->sd_addr + AZX_REG_ ## reg, val, cond, \
+			   delay_us, timeout_us)
+#define snd_hdac_stream_readl_poll(dev, reg, val, cond, delay_us, timeout_us) \
+	readl_poll_timeout((dev)->sd_addr + AZX_REG_ ## reg, val, cond, \
+			   delay_us, timeout_us)
 
 /* update a register, pass without AZX_REG_ prefix */
 #define snd_hdac_stream_updatel(dev, reg, mask, val) \
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index d26234f9ee46..03634ea198d0 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -188,12 +188,6 @@ void snd_hdac_ext_bus_link_power(struct hdac_device *codec, bool enable);
 #define snd_hdac_adsp_readq_poll(chip, reg, val, cond, delay_us, timeout_us) \
 	readq_poll_timeout((chip)->dsp_ba + (reg), val, cond, \
 			   delay_us, timeout_us)
-#define snd_hdac_stream_readb_poll(strm, reg, val, cond, delay_us, timeout_us) \
-	readb_poll_timeout((strm)->sd_addr + AZX_REG_ ## reg, val, cond, \
-			   delay_us, timeout_us)
-#define snd_hdac_stream_readl_poll(strm, reg, val, cond, delay_us, timeout_us) \
-	readl_poll_timeout((strm)->sd_addr + AZX_REG_ ## reg, val, cond, \
-			   delay_us, timeout_us)
 
 struct hdac_ext_device;
 
-- 
2.25.1

[PATCH 2/4] ALSA: hda: Rework snd_hdac_stream_reset() to use macros

[Amadeusz Sławiński]

We can use existing macros to poll and update register values instead of
open coding the functionality.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/hda/hdac_stream.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index f3582012d22f..ce6a2f270445 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -10,6 +10,7 @@
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/hdaudio.h>
+#include <sound/hdaudio_ext.h>
 #include <sound/hda_register.h>
 #include "trace.h"
 
@@ -165,7 +166,6 @@ EXPORT_SYMBOL_GPL(snd_hdac_stop_streams_and_chip);
 void snd_hdac_stream_reset(struct hdac_stream *azx_dev)
 {
 	unsigned char val;
-	int timeout;
 	int dma_run_state;
 
 	snd_hdac_stream_clear(azx_dev);
@@ -173,30 +173,17 @@ void snd_hdac_stream_reset(struct hdac_stream *azx_dev)
 	dma_run_state = snd_hdac_stream_readb(azx_dev, SD_CTL) & SD_CTL_DMA_START;
 
 	snd_hdac_stream_updateb(azx_dev, SD_CTL, 0, SD_CTL_STREAM_RESET);
-	udelay(3);
-	timeout = 300;
-	do {
-		val = snd_hdac_stream_readb(azx_dev, SD_CTL) &
-			SD_CTL_STREAM_RESET;
-		if (val)
-			break;
-	} while (--timeout);
+
+	/* wait for hardware to report that the stream entered reset */
+	snd_hdac_stream_readb_poll(azx_dev, SD_CTL, val, (val & SD_CTL_STREAM_RESET), 3, 300);
 
 	if (azx_dev->bus->dma_stop_delay && dma_run_state)
 		udelay(azx_dev->bus->dma_stop_delay);
 
-	val &= ~SD_CTL_STREAM_RESET;
-	snd_hdac_stream_writeb(azx_dev, SD_CTL, val);
-	udelay(3);
+	snd_hdac_stream_updateb(azx_dev, SD_CTL, SD_CTL_STREAM_RESET, 0);
 
-	timeout = 300;
-	/* waiting for hardware to report that the stream is out of reset */
-	do {
-		val = snd_hdac_stream_readb(azx_dev, SD_CTL) &
-			SD_CTL_STREAM_RESET;
-		if (!val)
-			break;
-	} while (--timeout);
+	/* wait for hardware to report that the stream is out of reset */
+	snd_hdac_stream_readb_poll(azx_dev, SD_CTL, val, !(val & SD_CTL_STREAM_RESET), 3, 300);
 
 	/* reset first position - may not be synced with hw at this time */
 	if (azx_dev->posbuf)
-- 
2.25.1

Re: [PATCH 2/4] ALSA: hda: Rework snd_hdac_stream_reset() to use macros

[Amadeusz Sławiński]

On 8/17/2022 3:11 PM, Amadeusz Sławiński wrote:
> We can use existing macros to poll and update register values instead of
> open coding the functionality.
> 
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> ---
>   sound/hda/hdac_stream.c | 27 +++++++--------------------
>   1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
> index f3582012d22f..ce6a2f270445 100644
> --- a/sound/hda/hdac_stream.c
> +++ b/sound/hda/hdac_stream.c
> @@ -10,6 +10,7 @@
>   #include <sound/core.h>
>   #include <sound/pcm.h>
>   #include <sound/hdaudio.h>
> +#include <sound/hdaudio_ext.h>

Eh... and this include addition should not be necessary after I moved 
macros in previous patch. I will wait if there are any other comments 
and send v2 tomorrow.

>   #include <sound/hda_register.h>
>   #include "trace.h"
>   
> @@ -165,7 +166,6 @@ EXPORT_SYMBOL_GPL(snd_hdac_stop_streams_and_chip);
>   void snd_hdac_stream_reset(struct hdac_stream *azx_dev)
>   {
>   	unsigned char val;
> -	int timeout;
>   	int dma_run_state;
>   
>   	snd_hdac_stream_clear(azx_dev);
> @@ -173,30 +173,17 @@ void snd_hdac_stream_reset(struct hdac_stream *azx_dev)
>   	dma_run_state = snd_hdac_stream_readb(azx_dev, SD_CTL) & SD_CTL_DMA_START;
>   
>   	snd_hdac_stream_updateb(azx_dev, SD_CTL, 0, SD_CTL_STREAM_RESET);
> -	udelay(3);
> -	timeout = 300;
> -	do {
> -		val = snd_hdac_stream_readb(azx_dev, SD_CTL) &
> -			SD_CTL_STREAM_RESET;
> -		if (val)
> -			break;
> -	} while (--timeout);
> +
> +	/* wait for hardware to report that the stream entered reset */
> +	snd_hdac_stream_readb_poll(azx_dev, SD_CTL, val, (val & SD_CTL_STREAM_RESET), 3, 300);
>   
>   	if (azx_dev->bus->dma_stop_delay && dma_run_state)
>   		udelay(azx_dev->bus->dma_stop_delay);
>   
> -	val &= ~SD_CTL_STREAM_RESET;
> -	snd_hdac_stream_writeb(azx_dev, SD_CTL, val);
> -	udelay(3);
> +	snd_hdac_stream_updateb(azx_dev, SD_CTL, SD_CTL_STREAM_RESET, 0);
>   
> -	timeout = 300;
> -	/* waiting for hardware to report that the stream is out of reset */
> -	do {
> -		val = snd_hdac_stream_readb(azx_dev, SD_CTL) &
> -			SD_CTL_STREAM_RESET;
> -		if (!val)
> -			break;
> -	} while (--timeout);
> +	/* wait for hardware to report that the stream is out of reset */
> +	snd_hdac_stream_readb_poll(azx_dev, SD_CTL, val, !(val & SD_CTL_STREAM_RESET), 3, 300);
>   
>   	/* reset first position - may not be synced with hw at this time */
>   	if (azx_dev->posbuf)

[PATCH 3/4] ALSA: hda: Remove unused MAX_PIN_CONFIGS constant

[Amadeusz Sławiński]

Since it was introduced around v2.6.30 it was never used. Also HDA
specification does not mention any limitation on number of PIN
configurations.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/pci/hda/hda_sysfs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/pci/hda/hda_sysfs.c b/sound/pci/hda/hda_sysfs.c
index bf951c10ae61..69ebc37a4d6f 100644
--- a/sound/pci/hda/hda_sysfs.c
+++ b/sound/pci/hda/hda_sysfs.c
@@ -375,8 +375,6 @@ static ssize_t user_pin_configs_show(struct device *dev,
 	return pin_configs_show(codec, &codec->user_pins, buf);
 }
 
-#define MAX_PIN_CONFIGS		32
-
 static int parse_user_pin_configs(struct hda_codec *codec, const char *buf)
 {
 	int nid, cfg, err;
-- 
2.25.1

[PATCH 4/4] ALSA: hda: Remove unused defines

[Amadeusz Sławiński]

There is no need to keep unused defines in file.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/pci/hda/hda_intel.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index a77165bd92a9..7720978dc132 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -86,9 +86,6 @@ enum {
 #define INTEL_SCH_HDA_DEVC      0x78
 #define INTEL_SCH_HDA_DEVC_NOSNOOP       (0x1<<11)
 
-/* Define VIA HD Audio Device ID*/
-#define VIA_HDAC_DEVICE_ID		0x3288
-
 /* max number of SDs */
 /* ICH, ATI and VIA have 4 playback and 4 capture */
 #define ICH6_NUM_CAPTURE	4
@@ -102,10 +99,6 @@ enum {
 #define ATIHDMI_NUM_CAPTURE	0
 #define ATIHDMI_NUM_PLAYBACK	8
 
-/* TERA has 4 playback and 3 capture */
-#define TERA_NUM_CAPTURE	3
-#define TERA_NUM_PLAYBACK	4
-
 
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;
 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;
-- 
2.25.1

Re: [PATCH 0/4] ALSA: hda: Minor cleanups

[Cezary Rojewski]

On 2022-08-17 3:11 PM, Amadeusz Sławiński wrote:
> First two patches are small refactoring of code to use poll macros
> instead of open coding register checks.
> Following two patches remove unused defines from code.
> 
> Amadeusz Sławiński (4):
>    ALSA: hda: Move stream-register polling macros
>    ALSA: hda: Rework snd_hdac_stream_reset() to use macros
>    ALSA: hda: Remove unused MAX_PIN_CONFIGS constant
>    ALSA: hda: Remove unused defines


Reviewed by me before sending, plus from the test results patchset 
causes no regression in the avs-driver CI.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>

Re: [PATCH 0/4] ALSA: hda: Minor cleanups

[Cezary Rojewski]

On 2022-08-17 3:23 PM, Cezary Rojewski wrote:
> On 2022-08-17 3:11 PM, Amadeusz Sławiński wrote:
>> First two patches are small refactoring of code to use poll macros
>> instead of open coding register checks.
>> Following two patches remove unused defines from code.
>>
>> Amadeusz Sławiński (4):
>>    ALSA: hda: Move stream-register polling macros
>>    ALSA: hda: Rework snd_hdac_stream_reset() to use macros
>>    ALSA: hda: Remove unused MAX_PIN_CONFIGS constant
>>    ALSA: hda: Remove unused defines
> 
> 
> Reviewed by me before sending, plus from the test results patchset 
> causes no regression in the avs-driver CI.
> 
> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>


RESEND. Patch was blocked so my tag did not make it to the list.