All of lore.kernel.org
 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 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.