alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] ALSA/ASoC: hdac_ext cleanups
@ 2021-12-16 23:11 Pierre-Louis Bossart
  2021-12-16 23:11 ` [PATCH 1/2] ALSA/ASoC: hda: move/rename snd_hdac_ext_stop_streams to hdac_stream.c Pierre-Louis Bossart
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Pierre-Louis Bossart @ 2021-12-16 23:11 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, Cezary Rojewski, broonie, Pierre-Louis Bossart

One function moved since it doesn't use the hdac_ext parts, and
renaming of variables to identify when hdac_stream and
hdac_ext_streams are used.

No functionality change.

Pierre-Louis Bossart (2):
  ALSA/ASoC: hda: move/rename snd_hdac_ext_stop_streams to hdac_stream.c
  ALSA: HDA: hdac_ext_stream: use consistent prefixes for variables

 include/sound/hdaudio.h         |   1 +
 include/sound/hdaudio_ext.h     |  27 ++--
 sound/hda/ext/hdac_ext_stream.c | 216 +++++++++++++++-----------------
 sound/hda/hdac_stream.c         |  16 +++
 sound/soc/intel/skylake/skl.c   |   4 +-
 5 files changed, 132 insertions(+), 132 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] ALSA/ASoC: hda: move/rename snd_hdac_ext_stop_streams to hdac_stream.c
  2021-12-16 23:11 [PATCH 0/2] ALSA/ASoC: hdac_ext cleanups Pierre-Louis Bossart
@ 2021-12-16 23:11 ` Pierre-Louis Bossart
  2021-12-17  9:18   ` Cezary Rojewski
  2021-12-16 23:11 ` [PATCH 2/2] ALSA: HDA: hdac_ext_stream: use consistent prefixes for variables Pierre-Louis Bossart
  2021-12-25  8:12 ` [PATCH 0/2] ALSA/ASoC: hdac_ext cleanups Takashi Iwai
  2 siblings, 1 reply; 5+ messages in thread
From: Pierre-Louis Bossart @ 2021-12-16 23:11 UTC (permalink / raw)
  To: alsa-devel
  Cc: Cezary Rojewski, tiwai, Pierre-Louis Bossart, Ranjani Sridharan,
	Kai Vehmanen, broonie, Péter Ujfalusi

snd_hdac_ext_stop_streams() has really nothing to do with the
extension, it just loops over the bus streams.

Move it to the hdac_stream layer and rename to remove the 'ext'
prefix and add the precision that the chip will also be stopped.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 include/sound/hdaudio.h         |  1 +
 include/sound/hdaudio_ext.h     |  1 -
 sound/hda/ext/hdac_ext_stream.c | 17 -----------------
 sound/hda/hdac_stream.c         | 16 ++++++++++++++++
 sound/soc/intel/skylake/skl.c   |  4 ++--
 5 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 22af68b01426..6a90ce405e60 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -558,6 +558,7 @@ int snd_hdac_stream_set_params(struct hdac_stream *azx_dev,
 void snd_hdac_stream_start(struct hdac_stream *azx_dev, bool fresh_start);
 void snd_hdac_stream_clear(struct hdac_stream *azx_dev);
 void snd_hdac_stream_stop(struct hdac_stream *azx_dev);
+void snd_hdac_stop_streams_and_chip(struct hdac_bus *bus);
 void snd_hdac_stream_reset(struct hdac_stream *azx_dev);
 void snd_hdac_stream_sync_trigger(struct hdac_stream *azx_dev, bool set,
 				  unsigned int streams, unsigned int reg);
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index d4e31ea16aba..56ea5cde5e63 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -92,7 +92,6 @@ void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus,
 				  struct hdac_ext_stream *azx_dev, bool decouple);
 void snd_hdac_ext_stream_decouple(struct hdac_bus *bus,
 				struct hdac_ext_stream *azx_dev, bool decouple);
-void snd_hdac_ext_stop_streams(struct hdac_bus *bus);
 
 int snd_hdac_ext_stream_set_spib(struct hdac_bus *bus,
 				 struct hdac_ext_stream *stream, u32 value);
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index 37154ed43bd5..c09652da43ff 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -475,23 +475,6 @@ int snd_hdac_ext_stream_get_spbmaxfifo(struct hdac_bus *bus,
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_get_spbmaxfifo);
 
-
-/**
- * snd_hdac_ext_stop_streams - stop all stream if running
- * @bus: HD-audio core bus
- */
-void snd_hdac_ext_stop_streams(struct hdac_bus *bus)
-{
-	struct hdac_stream *stream;
-
-	if (bus->chip_init) {
-		list_for_each_entry(stream, &bus->stream_list, list)
-			snd_hdac_stream_stop(stream);
-		snd_hdac_bus_stop_chip(bus);
-	}
-}
-EXPORT_SYMBOL_GPL(snd_hdac_ext_stop_streams);
-
 /**
  * snd_hdac_ext_stream_drsm_enable - enable DMA resume for a stream
  * @bus: HD-audio core bus
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index 9867555883c3..75986b89dcd9 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -142,6 +142,22 @@ void snd_hdac_stream_stop(struct hdac_stream *azx_dev)
 }
 EXPORT_SYMBOL_GPL(snd_hdac_stream_stop);
 
+/**
+ * snd_hdac_stop_streams_and_chip - stop all streams and chip if running
+ * @bus: HD-audio core bus
+ */
+void snd_hdac_stop_streams_and_chip(struct hdac_bus *bus)
+{
+	struct hdac_stream *stream;
+
+	if (bus->chip_init) {
+		list_for_each_entry(stream, &bus->stream_list, list)
+			snd_hdac_stream_stop(stream);
+		snd_hdac_bus_stop_chip(bus);
+	}
+}
+EXPORT_SYMBOL_GPL(snd_hdac_stop_streams_and_chip);
+
 /**
  * snd_hdac_stream_reset - reset a stream
  * @azx_dev: HD-audio core stream to reset
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 5b1a15e39912..148ddf4cace0 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -439,7 +439,7 @@ static int skl_free(struct hdac_bus *bus)
 
 	skl->init_done = 0; /* to be sure */
 
-	snd_hdac_ext_stop_streams(bus);
+	snd_hdac_stop_streams_and_chip(bus);
 
 	if (bus->irq >= 0)
 		free_irq(bus->irq, (void *)bus);
@@ -1096,7 +1096,7 @@ static void skl_shutdown(struct pci_dev *pci)
 	if (!skl->init_done)
 		return;
 
-	snd_hdac_ext_stop_streams(bus);
+	snd_hdac_stop_streams_and_chip(bus);
 	list_for_each_entry(s, &bus->stream_list, list) {
 		stream = stream_to_hdac_ext_stream(s);
 		snd_hdac_ext_stream_decouple(bus, stream, false);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] ALSA: HDA: hdac_ext_stream: use consistent prefixes for variables
  2021-12-16 23:11 [PATCH 0/2] ALSA/ASoC: hdac_ext cleanups Pierre-Louis Bossart
  2021-12-16 23:11 ` [PATCH 1/2] ALSA/ASoC: hda: move/rename snd_hdac_ext_stop_streams to hdac_stream.c Pierre-Louis Bossart
@ 2021-12-16 23:11 ` Pierre-Louis Bossart
  2021-12-25  8:12 ` [PATCH 0/2] ALSA/ASoC: hdac_ext cleanups Takashi Iwai
  2 siblings, 0 replies; 5+ messages in thread
From: Pierre-Louis Bossart @ 2021-12-16 23:11 UTC (permalink / raw)
  To: alsa-devel
  Cc: Cezary Rojewski, Kai Vehmanen, tiwai, Pierre-Louis Bossart,
	Rander Wang, broonie

The existing code maximizes confusion by using 'stream' and 'hstream'
variables of different types. Examples:

struct hdac_stream *stream;
struct hdac_ext_stream *stream;
struct hdac_stream *hstream;
struct hdac_ext_stream *hstream;

with some additional copy/paste remains:
struct hdac_ext_stream *azx_dev;

This patch suggests a consistent naming across all 'hdac_ext_stream'
functions. The convention is:

struct hdac_stream *hstream;
struct hdac_ext_stream *hext_stream;

No functionality change - just renaming of variables and more
consistent indentation.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
---
 include/sound/hdaudio_ext.h     |  26 ++---
 sound/hda/ext/hdac_ext_stream.c | 199 ++++++++++++++++----------------
 2 files changed, 113 insertions(+), 112 deletions(-)

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index 56ea5cde5e63..77123c3e4095 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -78,35 +78,35 @@ struct hdac_ext_stream {
 	container_of(s, struct hdac_ext_stream, hstream)
 
 void snd_hdac_ext_stream_init(struct hdac_bus *bus,
-				struct hdac_ext_stream *stream, int idx,
-				int direction, int tag);
+			      struct hdac_ext_stream *hext_stream, int idx,
+			      int direction, int tag);
 int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx,
-		int num_stream, int dir);
+				 int num_stream, int dir);
 void snd_hdac_stream_free_all(struct hdac_bus *bus);
 void snd_hdac_link_free_all(struct hdac_bus *bus);
 struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus,
 					   struct snd_pcm_substream *substream,
 					   int type);
-void snd_hdac_ext_stream_release(struct hdac_ext_stream *azx_dev, int type);
+void snd_hdac_ext_stream_release(struct hdac_ext_stream *hext_stream, int type);
 void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus,
-				  struct hdac_ext_stream *azx_dev, bool decouple);
+					 struct hdac_ext_stream *hext_stream, bool decouple);
 void snd_hdac_ext_stream_decouple(struct hdac_bus *bus,
 				struct hdac_ext_stream *azx_dev, bool decouple);
 
 int snd_hdac_ext_stream_set_spib(struct hdac_bus *bus,
-				 struct hdac_ext_stream *stream, u32 value);
+				 struct hdac_ext_stream *hext_stream, u32 value);
 int snd_hdac_ext_stream_get_spbmaxfifo(struct hdac_bus *bus,
-				 struct hdac_ext_stream *stream);
+				       struct hdac_ext_stream *hext_stream);
 void snd_hdac_ext_stream_drsm_enable(struct hdac_bus *bus,
 				bool enable, int index);
 int snd_hdac_ext_stream_set_dpibr(struct hdac_bus *bus,
-				struct hdac_ext_stream *stream, u32 value);
-int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *stream, u32 value);
+				struct hdac_ext_stream *hext_stream, u32 value);
+int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *hext_stream, u32 value);
 
-void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *hstream);
-void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *hstream);
-void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *hstream);
-int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *stream, int fmt);
+void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *hext_stream);
+void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *hext_stream);
+void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *hext_stream);
+int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *hext_stream, int fmt);
 
 struct hdac_ext_link {
 	struct hdac_bus *bus;
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index c09652da43ff..d2b5724b463f 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -18,7 +18,7 @@
 /**
  * snd_hdac_ext_stream_init - initialize each stream (aka device)
  * @bus: HD-audio core bus
- * @stream: HD-audio ext core stream object to initialize
+ * @hext_stream: HD-audio ext core stream object to initialize
  * @idx: stream index number
  * @direction: stream direction (SNDRV_PCM_STREAM_PLAYBACK or SNDRV_PCM_STREAM_CAPTURE)
  * @tag: the tag id to assign
@@ -27,34 +27,34 @@
  * invoke hdac stream initialization routine
  */
 void snd_hdac_ext_stream_init(struct hdac_bus *bus,
-				struct hdac_ext_stream *stream,
-				int idx, int direction, int tag)
+			      struct hdac_ext_stream *hext_stream,
+			      int idx, int direction, int tag)
 {
 	if (bus->ppcap) {
-		stream->pphc_addr = bus->ppcap + AZX_PPHC_BASE +
+		hext_stream->pphc_addr = bus->ppcap + AZX_PPHC_BASE +
 				AZX_PPHC_INTERVAL * idx;
 
-		stream->pplc_addr = bus->ppcap + AZX_PPLC_BASE +
+		hext_stream->pplc_addr = bus->ppcap + AZX_PPLC_BASE +
 				AZX_PPLC_MULTI * bus->num_streams +
 				AZX_PPLC_INTERVAL * idx;
 	}
 
 	if (bus->spbcap) {
-		stream->spib_addr = bus->spbcap + AZX_SPB_BASE +
+		hext_stream->spib_addr = bus->spbcap + AZX_SPB_BASE +
 					AZX_SPB_INTERVAL * idx +
 					AZX_SPB_SPIB;
 
-		stream->fifo_addr = bus->spbcap + AZX_SPB_BASE +
+		hext_stream->fifo_addr = bus->spbcap + AZX_SPB_BASE +
 					AZX_SPB_INTERVAL * idx +
 					AZX_SPB_MAXFIFO;
 	}
 
 	if (bus->drsmcap)
-		stream->dpibr_addr = bus->drsmcap + AZX_DRSM_BASE +
+		hext_stream->dpibr_addr = bus->drsmcap + AZX_DRSM_BASE +
 					AZX_DRSM_INTERVAL * idx;
 
-	stream->decoupled = false;
-	snd_hdac_stream_init(bus, &stream->hstream, idx, direction, tag);
+	hext_stream->decoupled = false;
+	snd_hdac_stream_init(bus, &hext_stream->hstream, idx, direction, tag);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_init);
 
@@ -67,18 +67,18 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_init);
  * @dir: direction of streams
  */
 int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx,
-		int num_stream, int dir)
+				 int num_stream, int dir)
 {
 	int stream_tag = 0;
 	int i, tag, idx = start_idx;
 
 	for (i = 0; i < num_stream; i++) {
-		struct hdac_ext_stream *stream =
-				kzalloc(sizeof(*stream), GFP_KERNEL);
-		if (!stream)
+		struct hdac_ext_stream *hext_stream =
+				kzalloc(sizeof(*hext_stream), GFP_KERNEL);
+		if (!hext_stream)
 			return -ENOMEM;
 		tag = ++stream_tag;
-		snd_hdac_ext_stream_init(bus, stream, idx, dir, tag);
+		snd_hdac_ext_stream_init(bus, hext_stream, idx, dir, tag);
 		idx++;
 	}
 
@@ -95,22 +95,22 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_init_all);
 void snd_hdac_stream_free_all(struct hdac_bus *bus)
 {
 	struct hdac_stream *s, *_s;
-	struct hdac_ext_stream *stream;
+	struct hdac_ext_stream *hext_stream;
 
 	list_for_each_entry_safe(s, _s, &bus->stream_list, list) {
-		stream = stream_to_hdac_ext_stream(s);
-		snd_hdac_ext_stream_decouple(bus, stream, false);
+		hext_stream = stream_to_hdac_ext_stream(s);
+		snd_hdac_ext_stream_decouple(bus, hext_stream, false);
 		list_del(&s->list);
-		kfree(stream);
+		kfree(hext_stream);
 	}
 }
 EXPORT_SYMBOL_GPL(snd_hdac_stream_free_all);
 
 void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus,
-					 struct hdac_ext_stream *stream,
+					 struct hdac_ext_stream *hext_stream,
 					 bool decouple)
 {
-	struct hdac_stream *hstream = &stream->hstream;
+	struct hdac_stream *hstream = &hext_stream->hstream;
 	u32 val;
 	int mask = AZX_PPCTL_PROCEN(hstream->index);
 
@@ -121,76 +121,76 @@ void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus,
 	else if (!decouple && val)
 		snd_hdac_updatel(bus->ppcap, AZX_REG_PP_PPCTL, mask, 0);
 
-	stream->decoupled = decouple;
+	hext_stream->decoupled = decouple;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_decouple_locked);
 
 /**
  * snd_hdac_ext_stream_decouple - decouple the hdac stream
  * @bus: HD-audio core bus
- * @stream: HD-audio ext core stream object to initialize
+ * @hext_stream: HD-audio ext core stream object to initialize
  * @decouple: flag to decouple
  */
 void snd_hdac_ext_stream_decouple(struct hdac_bus *bus,
-				  struct hdac_ext_stream *stream, bool decouple)
+				  struct hdac_ext_stream *hext_stream, bool decouple)
 {
 	spin_lock_irq(&bus->reg_lock);
-	snd_hdac_ext_stream_decouple_locked(bus, stream, decouple);
+	snd_hdac_ext_stream_decouple_locked(bus, hext_stream, decouple);
 	spin_unlock_irq(&bus->reg_lock);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_decouple);
 
 /**
  * snd_hdac_ext_link_stream_start - start a stream
- * @stream: HD-audio ext core stream to start
+ * @hext_stream: HD-audio ext core stream to start
  */
-void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *stream)
+void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *hext_stream)
 {
-	snd_hdac_updatel(stream->pplc_addr, AZX_REG_PPLCCTL,
+	snd_hdac_updatel(hext_stream->pplc_addr, AZX_REG_PPLCCTL,
 			 AZX_PPLCCTL_RUN, AZX_PPLCCTL_RUN);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_start);
 
 /**
  * snd_hdac_ext_link_stream_clear - stop a stream DMA
- * @stream: HD-audio ext core stream to stop
+ * @hext_stream: HD-audio ext core stream to stop
  */
-void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *stream)
+void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *hext_stream)
 {
-	snd_hdac_updatel(stream->pplc_addr, AZX_REG_PPLCCTL, AZX_PPLCCTL_RUN, 0);
+	snd_hdac_updatel(hext_stream->pplc_addr, AZX_REG_PPLCCTL, AZX_PPLCCTL_RUN, 0);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_clear);
 
 /**
  * snd_hdac_ext_link_stream_reset - reset a stream
- * @stream: HD-audio ext core stream to reset
+ * @hext_stream: HD-audio ext core stream to reset
  */
-void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *stream)
+void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *hext_stream)
 {
 	unsigned char val;
 	int timeout;
 
-	snd_hdac_ext_link_stream_clear(stream);
+	snd_hdac_ext_link_stream_clear(hext_stream);
 
-	snd_hdac_updatel(stream->pplc_addr, AZX_REG_PPLCCTL,
+	snd_hdac_updatel(hext_stream->pplc_addr, AZX_REG_PPLCCTL,
 			 AZX_PPLCCTL_STRST, AZX_PPLCCTL_STRST);
 	udelay(3);
 	timeout = 50;
 	do {
-		val = readl(stream->pplc_addr + AZX_REG_PPLCCTL) &
+		val = readl(hext_stream->pplc_addr + AZX_REG_PPLCCTL) &
 				AZX_PPLCCTL_STRST;
 		if (val)
 			break;
 		udelay(3);
 	} while (--timeout);
 	val &= ~AZX_PPLCCTL_STRST;
-	writel(val, stream->pplc_addr + AZX_REG_PPLCCTL);
+	writel(val, hext_stream->pplc_addr + AZX_REG_PPLCCTL);
 	udelay(3);
 
 	timeout = 50;
 	/* waiting for hardware to report that the stream is out of reset */
 	do {
-		val = readl(stream->pplc_addr + AZX_REG_PPLCCTL) & AZX_PPLCCTL_STRST;
+		val = readl(hext_stream->pplc_addr + AZX_REG_PPLCCTL) & AZX_PPLCCTL_STRST;
 		if (!val)
 			break;
 		udelay(3);
@@ -201,24 +201,24 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_reset);
 
 /**
  * snd_hdac_ext_link_stream_setup -  set up the SD for streaming
- * @stream: HD-audio ext core stream to set up
+ * @hext_stream: HD-audio ext core stream to set up
  * @fmt: stream format
  */
-int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *stream, int fmt)
+int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *hext_stream, int fmt)
 {
-	struct hdac_stream *hstream = &stream->hstream;
+	struct hdac_stream *hstream = &hext_stream->hstream;
 	unsigned int val;
 
 	/* make sure the run bit is zero for SD */
-	snd_hdac_ext_link_stream_clear(stream);
+	snd_hdac_ext_link_stream_clear(hext_stream);
 	/* program the stream_tag */
-	val = readl(stream->pplc_addr + AZX_REG_PPLCCTL);
+	val = readl(hext_stream->pplc_addr + AZX_REG_PPLCCTL);
 	val = (val & ~AZX_PPLCCTL_STRM_MASK) |
 		(hstream->stream_tag << AZX_PPLCCTL_STRM_SHIFT);
-	writel(val, stream->pplc_addr + AZX_REG_PPLCCTL);
+	writel(val, hext_stream->pplc_addr + AZX_REG_PPLCCTL);
 
 	/* program the stream format */
-	writew(fmt, stream->pplc_addr + AZX_REG_PPLCFMT);
+	writew(fmt, hext_stream->pplc_addr + AZX_REG_PPLCFMT);
 
 	return 0;
 }
@@ -230,7 +230,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_setup);
  * @stream: stream id
  */
 void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link,
-				 int stream)
+				     int stream)
 {
 	snd_hdac_updatew(link->ml_addr, AZX_REG_ML_LOSIDV, (1 << stream), 1 << stream);
 }
@@ -250,10 +250,10 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_link_clear_stream_id);
 
 static struct hdac_ext_stream *
 hdac_ext_link_stream_assign(struct hdac_bus *bus,
-				struct snd_pcm_substream *substream)
+			    struct snd_pcm_substream *substream)
 {
 	struct hdac_ext_stream *res = NULL;
-	struct hdac_stream *stream = NULL;
+	struct hdac_stream *hstream = NULL;
 
 	if (!bus->ppcap) {
 		dev_err(bus->dev, "stream type not supported\n");
@@ -261,22 +261,22 @@ hdac_ext_link_stream_assign(struct hdac_bus *bus,
 	}
 
 	spin_lock_irq(&bus->reg_lock);
-	list_for_each_entry(stream, &bus->stream_list, list) {
-		struct hdac_ext_stream *hstream = container_of(stream,
-						struct hdac_ext_stream,
-						hstream);
-		if (stream->direction != substream->stream)
+	list_for_each_entry(hstream, &bus->stream_list, list) {
+		struct hdac_ext_stream *hext_stream = container_of(hstream,
+								 struct hdac_ext_stream,
+								 hstream);
+		if (hstream->direction != substream->stream)
 			continue;
 
 		/* check if decoupled stream and not in use is available */
-		if (hstream->decoupled && !hstream->link_locked) {
-			res = hstream;
+		if (hext_stream->decoupled && !hext_stream->link_locked) {
+			res = hext_stream;
 			break;
 		}
 
-		if (!hstream->link_locked) {
-			snd_hdac_ext_stream_decouple_locked(bus, hstream, true);
-			res = hstream;
+		if (!hext_stream->link_locked) {
+			snd_hdac_ext_stream_decouple_locked(bus, hext_stream, true);
+			res = hext_stream;
 			break;
 		}
 	}
@@ -290,10 +290,10 @@ hdac_ext_link_stream_assign(struct hdac_bus *bus,
 
 static struct hdac_ext_stream *
 hdac_ext_host_stream_assign(struct hdac_bus *bus,
-				struct snd_pcm_substream *substream)
+			    struct snd_pcm_substream *substream)
 {
 	struct hdac_ext_stream *res = NULL;
-	struct hdac_stream *stream = NULL;
+	struct hdac_stream *hstream = NULL;
 
 	if (!bus->ppcap) {
 		dev_err(bus->dev, "stream type not supported\n");
@@ -301,17 +301,17 @@ hdac_ext_host_stream_assign(struct hdac_bus *bus,
 	}
 
 	spin_lock_irq(&bus->reg_lock);
-	list_for_each_entry(stream, &bus->stream_list, list) {
-		struct hdac_ext_stream *hstream = container_of(stream,
-						struct hdac_ext_stream,
-						hstream);
-		if (stream->direction != substream->stream)
+	list_for_each_entry(hstream, &bus->stream_list, list) {
+		struct hdac_ext_stream *hext_stream = container_of(hstream,
+								 struct hdac_ext_stream,
+								 hstream);
+		if (hstream->direction != substream->stream)
 			continue;
 
-		if (!stream->opened) {
-			if (!hstream->decoupled)
-				snd_hdac_ext_stream_decouple_locked(bus, hstream, true);
-			res = hstream;
+		if (!hstream->opened) {
+			if (!hext_stream->decoupled)
+				snd_hdac_ext_stream_decouple_locked(bus, hext_stream, true);
+			res = hext_stream;
 			break;
 		}
 	}
@@ -346,16 +346,17 @@ struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus,
 					   struct snd_pcm_substream *substream,
 					   int type)
 {
-	struct hdac_ext_stream *hstream = NULL;
-	struct hdac_stream *stream = NULL;
+	struct hdac_ext_stream *hext_stream = NULL;
+	struct hdac_stream *hstream = NULL;
 
 	switch (type) {
 	case HDAC_EXT_STREAM_TYPE_COUPLED:
-		stream = snd_hdac_stream_assign(bus, substream);
-		if (stream)
-			hstream = container_of(stream,
-					struct hdac_ext_stream, hstream);
-		return hstream;
+		hstream = snd_hdac_stream_assign(bus, substream);
+		if (hstream)
+			hext_stream = container_of(hstream,
+						   struct hdac_ext_stream,
+						   hstream);
+		return hext_stream;
 
 	case HDAC_EXT_STREAM_TYPE_HOST:
 		return hdac_ext_host_stream_assign(bus, substream);
@@ -371,34 +372,34 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_assign);
 
 /**
  * snd_hdac_ext_stream_release - release the assigned stream
- * @stream: HD-audio ext core stream to release
+ * @hext_stream: HD-audio ext core stream to release
  * @type: type of stream (coupled, host or link stream)
  *
  * Release the stream that has been assigned by snd_hdac_ext_stream_assign().
  */
-void snd_hdac_ext_stream_release(struct hdac_ext_stream *stream, int type)
+void snd_hdac_ext_stream_release(struct hdac_ext_stream *hext_stream, int type)
 {
-	struct hdac_bus *bus = stream->hstream.bus;
+	struct hdac_bus *bus = hext_stream->hstream.bus;
 
 	switch (type) {
 	case HDAC_EXT_STREAM_TYPE_COUPLED:
-		snd_hdac_stream_release(&stream->hstream);
+		snd_hdac_stream_release(&hext_stream->hstream);
 		break;
 
 	case HDAC_EXT_STREAM_TYPE_HOST:
 		spin_lock_irq(&bus->reg_lock);
-		if (stream->decoupled && !stream->link_locked)
-			snd_hdac_ext_stream_decouple_locked(bus, stream, false);
+		if (hext_stream->decoupled && !hext_stream->link_locked)
+			snd_hdac_ext_stream_decouple_locked(bus, hext_stream, false);
 		spin_unlock_irq(&bus->reg_lock);
-		snd_hdac_stream_release(&stream->hstream);
+		snd_hdac_stream_release(&hext_stream->hstream);
 		break;
 
 	case HDAC_EXT_STREAM_TYPE_LINK:
 		spin_lock_irq(&bus->reg_lock);
-		if (stream->decoupled && !stream->hstream.opened)
-			snd_hdac_ext_stream_decouple_locked(bus, stream, false);
-		stream->link_locked = 0;
-		stream->link_substream = NULL;
+		if (hext_stream->decoupled && !hext_stream->hstream.opened)
+			snd_hdac_ext_stream_decouple_locked(bus, hext_stream, false);
+		hext_stream->link_locked = 0;
+		hext_stream->link_substream = NULL;
 		spin_unlock_irq(&bus->reg_lock);
 		break;
 
@@ -437,11 +438,11 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_spbcap_enable);
 /**
  * snd_hdac_ext_stream_set_spib - sets the spib value of a stream
  * @bus: HD-audio core bus
- * @stream: hdac_ext_stream
+ * @hext_stream: hdac_ext_stream
  * @value: spib value to set
  */
 int snd_hdac_ext_stream_set_spib(struct hdac_bus *bus,
-				 struct hdac_ext_stream *stream, u32 value)
+				 struct hdac_ext_stream *hext_stream, u32 value)
 {
 
 	if (!bus->spbcap) {
@@ -449,7 +450,7 @@ int snd_hdac_ext_stream_set_spib(struct hdac_bus *bus,
 		return -EINVAL;
 	}
 
-	writel(value, stream->spib_addr);
+	writel(value, hext_stream->spib_addr);
 
 	return 0;
 }
@@ -458,12 +459,12 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_spib);
 /**
  * snd_hdac_ext_stream_get_spbmaxfifo - gets the spib value of a stream
  * @bus: HD-audio core bus
- * @stream: hdac_ext_stream
+ * @hext_stream: hdac_ext_stream
  *
  * Return maxfifo for the stream
  */
 int snd_hdac_ext_stream_get_spbmaxfifo(struct hdac_bus *bus,
-				 struct hdac_ext_stream *stream)
+				 struct hdac_ext_stream *hext_stream)
 {
 
 	if (!bus->spbcap) {
@@ -471,7 +472,7 @@ int snd_hdac_ext_stream_get_spbmaxfifo(struct hdac_bus *bus,
 		return -EINVAL;
 	}
 
-	return readl(stream->fifo_addr);
+	return readl(hext_stream->fifo_addr);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_get_spbmaxfifo);
 
@@ -503,11 +504,11 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_drsm_enable);
 /**
  * snd_hdac_ext_stream_set_dpibr - sets the dpibr value of a stream
  * @bus: HD-audio core bus
- * @stream: hdac_ext_stream
+ * @hext_stream: hdac_ext_stream
  * @value: dpib value to set
  */
 int snd_hdac_ext_stream_set_dpibr(struct hdac_bus *bus,
-				 struct hdac_ext_stream *stream, u32 value)
+				  struct hdac_ext_stream *hext_stream, u32 value)
 {
 
 	if (!bus->drsmcap) {
@@ -515,7 +516,7 @@ int snd_hdac_ext_stream_set_dpibr(struct hdac_bus *bus,
 		return -EINVAL;
 	}
 
-	writel(value, stream->dpibr_addr);
+	writel(value, hext_stream->dpibr_addr);
 
 	return 0;
 }
@@ -523,12 +524,12 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_dpibr);
 
 /**
  * snd_hdac_ext_stream_set_lpib - sets the lpib value of a stream
- * @stream: hdac_ext_stream
+ * @hext_stream: hdac_ext_stream
  * @value: lpib value to set
  */
-int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *stream, u32 value)
+int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *hext_stream, u32 value)
 {
-	snd_hdac_stream_writel(&stream->hstream, SD_LPIB, value);
+	snd_hdac_stream_writel(&hext_stream->hstream, SD_LPIB, value);
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] ALSA/ASoC: hda: move/rename snd_hdac_ext_stop_streams to hdac_stream.c
  2021-12-16 23:11 ` [PATCH 1/2] ALSA/ASoC: hda: move/rename snd_hdac_ext_stop_streams to hdac_stream.c Pierre-Louis Bossart
@ 2021-12-17  9:18   ` Cezary Rojewski
  0 siblings, 0 replies; 5+ messages in thread
From: Cezary Rojewski @ 2021-12-17  9:18 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel
  Cc: tiwai, Kai Vehmanen, broonie, Péter Ujfalusi, Ranjani Sridharan

On 2021-12-17 12:11 AM, Pierre-Louis Bossart wrote:
> snd_hdac_ext_stop_streams() has really nothing to do with the
> extension, it just loops over the bus streams.
> 
> Move it to the hdac_stream layer and rename to remove the 'ext'
> prefix and add the precision that the chip will also be stopped.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Kai Vehmanen <kai.vehmanen@intel.com>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Change looks good. I wonder though, if there is a better 'name' for the 
process (here, stop streams + stop chip as a tear down process) than 
'stop_streams_and_chip'.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] ALSA/ASoC: hdac_ext cleanups
  2021-12-16 23:11 [PATCH 0/2] ALSA/ASoC: hdac_ext cleanups Pierre-Louis Bossart
  2021-12-16 23:11 ` [PATCH 1/2] ALSA/ASoC: hda: move/rename snd_hdac_ext_stop_streams to hdac_stream.c Pierre-Louis Bossart
  2021-12-16 23:11 ` [PATCH 2/2] ALSA: HDA: hdac_ext_stream: use consistent prefixes for variables Pierre-Louis Bossart
@ 2021-12-25  8:12 ` Takashi Iwai
  2 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2021-12-25  8:12 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: alsa-devel, broonie, Cezary Rojewski

On Fri, 17 Dec 2021 00:11:26 +0100,
Pierre-Louis Bossart wrote:
> 
> One function moved since it doesn't use the hdac_ext parts, and
> renaming of variables to identify when hdac_stream and
> hdac_ext_streams are used.
> 
> No functionality change.
> 
> Pierre-Louis Bossart (2):
>   ALSA/ASoC: hda: move/rename snd_hdac_ext_stop_streams to hdac_stream.c
>   ALSA: HDA: hdac_ext_stream: use consistent prefixes for variables

Applied both patches now.  Thanks.


Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-25  8:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 23:11 [PATCH 0/2] ALSA/ASoC: hdac_ext cleanups Pierre-Louis Bossart
2021-12-16 23:11 ` [PATCH 1/2] ALSA/ASoC: hda: move/rename snd_hdac_ext_stop_streams to hdac_stream.c Pierre-Louis Bossart
2021-12-17  9:18   ` Cezary Rojewski
2021-12-16 23:11 ` [PATCH 2/2] ALSA: HDA: hdac_ext_stream: use consistent prefixes for variables Pierre-Louis Bossart
2021-12-25  8:12 ` [PATCH 0/2] ALSA/ASoC: hdac_ext cleanups Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).