[PATCH 0/3] ALSA: hda: bus cleanup

[PATCH 0/3] ALSA: hda: bus cleanup

[Takashi Iwai]

Hi,

this is a few patches to simplify and cleanup the HD-audio bus ops.

The first two patches translate the indirect calls of DMA page
allocation and MMIO accesses with the direct ones, as well as
eliminating the whole bus->io_ops.

The last one is SOF-specific, and fixes/cleans up by calling the
proper hdaudio bus init function, as formerly discussed.


Takashi

===

Takashi Iwai (3):
  ALSA: hda: Remove page allocation redirection
  ALSA: hda: Direct MMIO accesses
  ASoC: SOF: Intel: Initialize hdaudio bus properly

 include/sound/hdaudio.h                | 69 +++++++++++++--------------
 include/sound/hdaudio_ext.h            |  1 -
 sound/hda/Kconfig                      |  3 ++
 sound/hda/ext/hdac_ext_bus.c           | 60 +-----------------------
 sound/hda/hdac_bus.c                   | 36 ++++++++++++--
 sound/hda/hdac_controller.c            | 18 +++----
 sound/hda/hdac_stream.c                |  8 ++--
 sound/pci/hda/Kconfig                  |  1 +
 sound/pci/hda/hda_controller.c         |  6 +--
 sound/pci/hda/hda_controller.h         |  3 +-
 sound/pci/hda/hda_intel.c              | 71 ++--------------------------
 sound/pci/hda/hda_tegra.c              | 84 +--------------------------------
 sound/soc/intel/skylake/skl-messages.c | 15 +-----
 sound/soc/intel/skylake/skl.c          |  7 ++-
 sound/soc/sof/intel/hda-bus.c          | 85 ++++------------------------------
 sound/soc/sof/intel/hda-dsp.c          |  2 +-
 sound/soc/sof/intel/hda.c              |  6 +--
 sound/soc/sof/intel/hda.h              |  3 +-
 18 files changed, 107 insertions(+), 371 deletions(-)

-- 
2.16.4

[PATCH 1/3] ALSA: hda: Remove page allocation redirection

[Takashi Iwai]

The HD-audio core allocates and release pages via driver's specific
dma_alloc_pages and dma_free_pages ops defined in bus->io_ops.  This
was because some platforms require the uncached pages and the handling
of page flags had to be done locally in the driver code.

Since the recent change in ALSA core memory allocator, we can simply
pass SNDRV_DMA_TYPE_DEV_UC for the uncached pages, and the only
difference became about this type to be passed to the core allocator.
That is, it's good time for cleaning up the mess.

This patch changes the allocation code in HD-audio core to call the
core allocator directly so that we get rid of dma_alloc_pages and
dma_free_pages io_ops.  If a driver needs the uncached pages, it has
to set bus->dma_type right after the bus initialization.

This is merely a code refactoring and shouldn't bring any behavior
changes.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/hdaudio.h                |  6 +-----
 sound/hda/ext/hdac_ext_bus.c           | 13 -------------
 sound/hda/hdac_bus.c                   |  1 +
 sound/hda/hdac_controller.c            | 18 +++++++++---------
 sound/hda/hdac_stream.c                |  8 ++++----
 sound/pci/hda/hda_intel.c              | 24 ++++--------------------
 sound/pci/hda/hda_tegra.c              | 16 ----------------
 sound/soc/intel/skylake/skl-messages.c | 15 ++-------------
 sound/soc/sof/intel/hda-bus.c          | 14 --------------
 9 files changed, 21 insertions(+), 94 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 612a17e375d0..20549def0a27 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -264,11 +264,6 @@ struct hdac_io_ops {
 	u16 (*reg_readw)(u16 __iomem *addr);
 	void (*reg_writeb)(u8 value, u8 __iomem *addr);
 	u8 (*reg_readb)(u8 __iomem *addr);
-	/* Allocation ops */
-	int (*dma_alloc_pages)(struct hdac_bus *bus, int type, size_t size,
-			       struct snd_dma_buffer *buf);
-	void (*dma_free_pages)(struct hdac_bus *bus,
-			       struct snd_dma_buffer *buf);
 };
 
 #define HDA_UNSOL_QUEUE_SIZE	64
@@ -344,6 +339,7 @@ struct hdac_bus {
 	/* CORB/RIRB and position buffers */
 	struct snd_dma_buffer rb;
 	struct snd_dma_buffer posbuf;
+	int dma_type;			/* SNDRV_DMA_TYPE_XXX for CORB/RIRB */
 
 	/* hdac_stream linked list */
 	struct list_head stream_list;
diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
index 4f9f1d2a2ec5..7825b74068f4 100644
--- a/sound/hda/ext/hdac_ext_bus.c
+++ b/sound/hda/ext/hdac_ext_bus.c
@@ -47,17 +47,6 @@ static u8 hdac_ext_readb(u8 __iomem *addr)
 	return readb(addr);
 }
 
-static int hdac_ext_dma_alloc_pages(struct hdac_bus *bus, int type,
-			   size_t size, struct snd_dma_buffer *buf)
-{
-	return snd_dma_alloc_pages(type, bus->dev, size, buf);
-}
-
-static void hdac_ext_dma_free_pages(struct hdac_bus *bus, struct snd_dma_buffer *buf)
-{
-	snd_dma_free_pages(buf);
-}
-
 static const struct hdac_io_ops hdac_ext_default_io = {
 	.reg_writel = hdac_ext_writel,
 	.reg_readl = hdac_ext_readl,
@@ -65,8 +54,6 @@ static const struct hdac_io_ops hdac_ext_default_io = {
 	.reg_readw = hdac_ext_readw,
 	.reg_writeb = hdac_ext_writeb,
 	.reg_readb = hdac_ext_readb,
-	.dma_alloc_pages = hdac_ext_dma_alloc_pages,
-	.dma_free_pages = hdac_ext_dma_free_pages,
 };
 
 /**
diff --git a/sound/hda/hdac_bus.c b/sound/hda/hdac_bus.c
index 14e57ffd5bc1..00ea12e67dc8 100644
--- a/sound/hda/hdac_bus.c
+++ b/sound/hda/hdac_bus.c
@@ -34,6 +34,7 @@ int snd_hdac_bus_init(struct hdac_bus *bus, struct device *dev,
 	else
 		bus->ops = &default_ops;
 	bus->io_ops = io_ops;
+	bus->dma_type = SNDRV_DMA_TYPE_DEV;
 	INIT_LIST_HEAD(&bus->stream_list);
 	INIT_LIST_HEAD(&bus->codec_list);
 	INIT_WORK(&bus->unsol_work, snd_hdac_bus_process_unsol_events);
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index 3b0110545070..7e7be8e4dcf9 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -575,12 +575,13 @@ int snd_hdac_bus_alloc_stream_pages(struct hdac_bus *bus)
 {
 	struct hdac_stream *s;
 	int num_streams = 0;
+	int dma_type = bus->dma_type ? bus->dma_type : SNDRV_DMA_TYPE_DEV;
 	int err;
 
 	list_for_each_entry(s, &bus->stream_list, list) {
 		/* allocate memory for the BDL for each stream */
-		err = bus->io_ops->dma_alloc_pages(bus, SNDRV_DMA_TYPE_DEV,
-						   BDL_SIZE, &s->bdl);
+		err = snd_dma_alloc_pages(dma_type, bus->dev,
+					  BDL_SIZE, &s->bdl);
 		num_streams++;
 		if (err < 0)
 			return -ENOMEM;
@@ -589,16 +590,15 @@ int snd_hdac_bus_alloc_stream_pages(struct hdac_bus *bus)
 	if (WARN_ON(!num_streams))
 		return -EINVAL;
 	/* allocate memory for the position buffer */
-	err = bus->io_ops->dma_alloc_pages(bus, SNDRV_DMA_TYPE_DEV,
-					   num_streams * 8, &bus->posbuf);
+	err = snd_dma_alloc_pages(dma_type, bus->dev,
+				  num_streams * 8, &bus->posbuf);
 	if (err < 0)
 		return -ENOMEM;
 	list_for_each_entry(s, &bus->stream_list, list)
 		s->posbuf = (__le32 *)(bus->posbuf.area + s->index * 8);
 
 	/* single page (at least 4096 bytes) must suffice for both ringbuffes */
-	return bus->io_ops->dma_alloc_pages(bus, SNDRV_DMA_TYPE_DEV,
-					    PAGE_SIZE, &bus->rb);
+	return snd_dma_alloc_pages(dma_type, bus->dev, PAGE_SIZE, &bus->rb);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_alloc_stream_pages);
 
@@ -612,12 +612,12 @@ void snd_hdac_bus_free_stream_pages(struct hdac_bus *bus)
 
 	list_for_each_entry(s, &bus->stream_list, list) {
 		if (s->bdl.area)
-			bus->io_ops->dma_free_pages(bus, &s->bdl);
+			snd_dma_free_pages(&s->bdl);
 	}
 
 	if (bus->rb.area)
-		bus->io_ops->dma_free_pages(bus, &bus->rb);
+		snd_dma_free_pages(&bus->rb);
 	if (bus->posbuf.area)
-		bus->io_ops->dma_free_pages(bus, &bus->posbuf);
+		snd_dma_free_pages(&bus->posbuf);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_free_stream_pages);
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index 55d53b89ac21..fc68d4ce0a37 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -680,8 +680,8 @@ int snd_hdac_dsp_prepare(struct hdac_stream *azx_dev, unsigned int format,
 	azx_dev->locked = true;
 	spin_unlock_irq(&bus->reg_lock);
 
-	err = bus->io_ops->dma_alloc_pages(bus, SNDRV_DMA_TYPE_DEV_SG,
-					   byte_size, bufp);
+	err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, bus->dev,
+				  byte_size, bufp);
 	if (err < 0)
 		goto err_alloc;
 
@@ -707,7 +707,7 @@ int snd_hdac_dsp_prepare(struct hdac_stream *azx_dev, unsigned int format,
 	return azx_dev->stream_tag;
 
  error:
-	bus->io_ops->dma_free_pages(bus, bufp);
+	snd_dma_free_pages(bufp);
  err_alloc:
 	spin_lock_irq(&bus->reg_lock);
 	azx_dev->locked = false;
@@ -754,7 +754,7 @@ void snd_hdac_dsp_cleanup(struct hdac_stream *azx_dev,
 	azx_dev->period_bytes = 0;
 	azx_dev->format_val = 0;
 
-	bus->io_ops->dma_free_pages(bus, dmab);
+	snd_dma_free_pages(dmab);
 	dmab->area = NULL;
 
 	spin_lock_irq(&bus->reg_lock);
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index cb8b0945547c..3bb4c26f2799 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1694,6 +1694,10 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
 		return err;
 	}
 
+	/* use the non-cached pages in non-snoop mode */
+	if (!azx_snoop(chip))
+		azx_bus(chip)->dma_type = SNDRV_DMA_TYPE_DEV_UC;
+
 	/* Workaround for a communication error on CFL (bko#199007) and CNL */
 	if (IS_CFL(pci) || IS_CNL(pci))
 		azx_bus(chip)->polling_mode = 1;
@@ -1979,24 +1983,6 @@ static int disable_msi_reset_irq(struct azx *chip)
 	return 0;
 }
 
-/* DMA page allocation helpers.  */
-static int dma_alloc_pages(struct hdac_bus *bus,
-			   int type,
-			   size_t size,
-			   struct snd_dma_buffer *buf)
-{
-	struct azx *chip = bus_to_azx(bus);
-
-	if (!azx_snoop(chip) && type == SNDRV_DMA_TYPE_DEV)
-		type = SNDRV_DMA_TYPE_DEV_UC;
-	return snd_dma_alloc_pages(type, bus->dev, size, buf);
-}
-
-static void dma_free_pages(struct hdac_bus *bus, struct snd_dma_buffer *buf)
-{
-	snd_dma_free_pages(buf);
-}
-
 static void pcm_mmap_prepare(struct snd_pcm_substream *substream,
 			     struct vm_area_struct *area)
 {
@@ -2015,8 +2001,6 @@ static const struct hdac_io_ops pci_hda_io_ops = {
 	.reg_readw = pci_azx_readw,
 	.reg_writeb = pci_azx_writeb,
 	.reg_readb = pci_azx_readb,
-	.dma_alloc_pages = dma_alloc_pages,
-	.dma_free_pages = dma_free_pages,
 };
 
 static const struct hda_controller_ops pci_hda_ops = {
diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 7dbe9f39fc79..ba414cc639f1 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -75,20 +75,6 @@ MODULE_PARM_DESC(power_save,
 #define power_save	0
 #endif
 
-/*
- * DMA page allocation ops.
- */
-static int dma_alloc_pages(struct hdac_bus *bus, int type, size_t size,
-			   struct snd_dma_buffer *buf)
-{
-	return snd_dma_alloc_pages(type, bus->dev, size, buf);
-}
-
-static void dma_free_pages(struct hdac_bus *bus, struct snd_dma_buffer *buf)
-{
-	snd_dma_free_pages(buf);
-}
-
 /*
  * Register access ops. Tegra HDA register access is DWORD only.
  */
@@ -153,8 +139,6 @@ static const struct hdac_io_ops hda_tegra_io_ops = {
 	.reg_readw = hda_tegra_readw,
 	.reg_writeb = hda_tegra_writeb,
 	.reg_readb = hda_tegra_readb,
-	.dma_alloc_pages = dma_alloc_pages,
-	.dma_free_pages = dma_free_pages,
 };
 
 static const struct hda_controller_ops hda_tegra_ops; /* nothing special */
diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index febc070839e0..c6f9e05c929e 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -25,23 +25,12 @@
 static int skl_alloc_dma_buf(struct device *dev,
 		struct snd_dma_buffer *dmab, size_t size)
 {
-	struct hdac_bus *bus = dev_get_drvdata(dev);
-
-	if (!bus)
-		return -ENODEV;
-
-	return  bus->io_ops->dma_alloc_pages(bus, SNDRV_DMA_TYPE_DEV, size, dmab);
+	return snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, dev, size, dmab);
 }
 
 static int skl_free_dma_buf(struct device *dev, struct snd_dma_buffer *dmab)
 {
-	struct hdac_bus *bus = dev_get_drvdata(dev);
-
-	if (!bus)
-		return -ENODEV;
-
-	bus->io_ops->dma_free_pages(bus, dmab);
-
+	snd_dma_free_pages(dmab);
 	return 0;
 }
 
diff --git a/sound/soc/sof/intel/hda-bus.c b/sound/soc/sof/intel/hda-bus.c
index a7e6d8227df6..0bc93fa06b5b 100644
--- a/sound/soc/sof/intel/hda-bus.c
+++ b/sound/soc/sof/intel/hda-bus.c
@@ -51,18 +51,6 @@ static u8 sof_hda_readb(u8 __iomem *addr)
 	return readb(addr);
 }
 
-static int sof_hda_dma_alloc_pages(struct hdac_bus *bus, int type,
-				   size_t size, struct snd_dma_buffer *buf)
-{
-	return snd_dma_alloc_pages(type, bus->dev, size, buf);
-}
-
-static void sof_hda_dma_free_pages(struct hdac_bus *bus,
-				   struct snd_dma_buffer *buf)
-{
-	snd_dma_free_pages(buf);
-}
-
 static const struct hdac_io_ops io_ops = {
 	.reg_writel = sof_hda_writel,
 	.reg_readl = sof_hda_readl,
@@ -70,8 +58,6 @@ static const struct hdac_io_ops io_ops = {
 	.reg_readw = sof_hda_readw,
 	.reg_writeb = sof_hda_writeb,
 	.reg_readb = sof_hda_readb,
-	.dma_alloc_pages = sof_hda_dma_alloc_pages,
-	.dma_free_pages = sof_hda_dma_free_pages,
 };
 
 /*
-- 
2.16.4

[PATCH 2/3] ALSA: hda: Direct MMIO accesses

[Takashi Iwai]

HD-audio drivers access to the mmio registers indirectly via the
corresponding bus->io_ops callbacks.  This is because some platform
(notably Tegra SoC) requires the word-aligned access.  But it's rather
a rare case, and other platforms suffer from the penalties by indirect
calls unnecessarily.

This patch is an attempt to optimize and cleanup for this situation.
Now the special aligned access is used only when a new kconfig
CONFIG_SND_HDA_ALIGNED_MMIO is set.  And the HD-audio core itself
provides the aligned MMIO access helpers instead of the driver side.
If Kconfig isn't set (as default), the standard helpers like readl()
or writel() are used directly.

A couple of places in ASoC Intel drivers have the access via io_ops
reg_writel(), and they are replaced with the direct writel() calls.

And now with this patch, the whole bus->io_ops becomes empty, so it's
dropped completely.  The bus initialization functions are changed
accordingly as well to drop the whole bus->io_ops.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/hdaudio.h        | 63 +++++++++++++++++++-------------------
 include/sound/hdaudio_ext.h    |  1 -
 sound/hda/Kconfig              |  3 ++
 sound/hda/ext/hdac_ext_bus.c   | 47 +----------------------------
 sound/hda/hdac_bus.c           | 35 +++++++++++++++++++---
 sound/pci/hda/Kconfig          |  1 +
 sound/pci/hda/hda_controller.c |  6 ++--
 sound/pci/hda/hda_controller.h |  3 +-
 sound/pci/hda/hda_intel.c      | 47 +----------------------------
 sound/pci/hda/hda_tegra.c      | 68 +-----------------------------------------
 sound/soc/intel/skylake/skl.c  |  7 ++---
 sound/soc/sof/intel/hda-bus.c  | 40 -------------------------
 sound/soc/sof/intel/hda-dsp.c  |  2 +-
 13 files changed, 75 insertions(+), 248 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 20549def0a27..4af4af55e854 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -253,19 +253,6 @@ struct hdac_ext_bus_ops {
 	int (*hdev_detach)(struct hdac_device *hdev);
 };
 
-/*
- * Lowlevel I/O operators
- */
-struct hdac_io_ops {
-	/* mapped register accesses */
-	void (*reg_writel)(u32 value, u32 __iomem *addr);
-	u32 (*reg_readl)(u32 __iomem *addr);
-	void (*reg_writew)(u16 value, u16 __iomem *addr);
-	u16 (*reg_readw)(u16 __iomem *addr);
-	void (*reg_writeb)(u8 value, u8 __iomem *addr);
-	u8 (*reg_readb)(u8 __iomem *addr);
-};
-
 #define HDA_UNSOL_QUEUE_SIZE	64
 #define HDA_MAX_CODECS		8	/* limit by controller side */
 
@@ -299,7 +286,6 @@ struct hdac_rb {
 struct hdac_bus {
 	struct device *dev;
 	const struct hdac_bus_ops *ops;
-	const struct hdac_io_ops *io_ops;
 	const struct hdac_ext_bus_ops *ext_ops;
 
 	/* h/w resources */
@@ -380,8 +366,7 @@ struct hdac_bus {
 };
 
 int snd_hdac_bus_init(struct hdac_bus *bus, struct device *dev,
-		      const struct hdac_bus_ops *ops,
-		      const struct hdac_io_ops *io_ops);
+		      const struct hdac_bus_ops *ops);
 void snd_hdac_bus_exit(struct hdac_bus *bus);
 int snd_hdac_bus_exec_verb(struct hdac_bus *bus, unsigned int addr,
 			   unsigned int cmd, unsigned int *res);
@@ -425,21 +410,38 @@ int snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
 int snd_hdac_bus_alloc_stream_pages(struct hdac_bus *bus);
 void snd_hdac_bus_free_stream_pages(struct hdac_bus *bus);
 
+#ifdef CONFIG_SND_HDA_ALIGNED_MMIO
+unsigned int snd_hdac_aligned_read(void __iomem *addr, unsigned int mask);
+void snd_hdac_aligned_write(unsigned int val, void __iomem *addr,
+			    unsigned int mask);
+#define snd_hdac_reg_writeb(v, addr)	snd_hdac_aligned_write(v, addr, 0xff)
+#define snd_hdac_reg_writew(v, addr)	snd_hdac_aligned_write(v, addr, 0xffff)
+#define snd_hdac_reg_readb(addr)	snd_hdac_aligned_read(addr, 0xff)
+#define snd_hdac_reg_readw(addr)	snd_hdac_aligned_read(addr, 0xffff)
+#else /* CONFIG_SND_HDA_ALIGNED_MMIO */
+#define snd_hdac_reg_writeb(val, addr)	writeb(val, addr)
+#define snd_hdac_reg_writew(val, addr)	writew(val, addr)
+#define snd_hdac_reg_readb(addr)	readb(addr)
+#define snd_hdac_reg_readw(addr)	readw(addr)
+#endif /* CONFIG_SND_HDA_ALIGNED_MMIO */
+#define snd_hdac_reg_writel(val, addr)	writel(val, addr)
+#define snd_hdac_reg_readl(addr)	readl(addr)
+
 /*
  * macros for easy use
  */
 #define _snd_hdac_chip_writeb(chip, reg, value) \
-	((chip)->io_ops->reg_writeb(value, (chip)->remap_addr + (reg)))
+	snd_hdac_reg_writeb(value, (chip)->remap_addr + (reg))
 #define _snd_hdac_chip_readb(chip, reg) \
-	((chip)->io_ops->reg_readb((chip)->remap_addr + (reg)))
+	snd_hdac_reg_readb((chip)->remap_addr + (reg))
 #define _snd_hdac_chip_writew(chip, reg, value) \
-	((chip)->io_ops->reg_writew(value, (chip)->remap_addr + (reg)))
+	snd_hdac_reg_writew(value, (chip)->remap_addr + (reg))
 #define _snd_hdac_chip_readw(chip, reg) \
-	((chip)->io_ops->reg_readw((chip)->remap_addr + (reg)))
+	snd_hdac_reg_readw((chip)->remap_addr + (reg))
 #define _snd_hdac_chip_writel(chip, reg, value) \
-	((chip)->io_ops->reg_writel(value, (chip)->remap_addr + (reg)))
+	snd_hdac_reg_writel(value, (chip)->remap_addr + (reg))
 #define _snd_hdac_chip_readl(chip, reg) \
-	((chip)->io_ops->reg_readl((chip)->remap_addr + (reg)))
+	snd_hdac_reg_readl((chip)->remap_addr + (reg))
 
 /* read/write a register, pass without AZX_REG_ prefix */
 #define snd_hdac_chip_writel(chip, reg, value) \
@@ -544,24 +546,19 @@ int snd_hdac_get_stream_stripe_ctl(struct hdac_bus *bus,
 /*
  * macros for easy use
  */
-#define _snd_hdac_stream_write(type, dev, reg, value)			\
-	((dev)->bus->io_ops->reg_write ## type(value, (dev)->sd_addr + (reg)))
-#define _snd_hdac_stream_read(type, dev, reg)				\
-	((dev)->bus->io_ops->reg_read ## type((dev)->sd_addr + (reg)))
-
 /* read/write a register, pass without AZX_REG_ prefix */
 #define snd_hdac_stream_writel(dev, reg, value) \
-	_snd_hdac_stream_write(l, dev, AZX_REG_ ## reg, value)
+	snd_hdac_reg_writel(value, (dev)->sd_addr + AZX_REG_ ## reg)
 #define snd_hdac_stream_writew(dev, reg, value) \
-	_snd_hdac_stream_write(w, dev, AZX_REG_ ## reg, value)
+	snd_hdac_reg_writew(value, (dev)->sd_addr + AZX_REG_ ## reg)
 #define snd_hdac_stream_writeb(dev, reg, value) \
-	_snd_hdac_stream_write(b, dev, AZX_REG_ ## reg, value)
+	snd_hdac_reg_writeb(value, (dev)->sd_addr + AZX_REG_ ## reg)
 #define snd_hdac_stream_readl(dev, reg) \
-	_snd_hdac_stream_read(l, dev, AZX_REG_ ## reg)
+	snd_hdac_reg_readl((dev)->sd_addr + AZX_REG_ ## reg)
 #define snd_hdac_stream_readw(dev, reg) \
-	_snd_hdac_stream_read(w, dev, AZX_REG_ ## reg)
+	snd_hdac_reg_readw((dev)->sd_addr + AZX_REG_ ## reg)
 #define snd_hdac_stream_readb(dev, reg) \
-	_snd_hdac_stream_read(b, dev, AZX_REG_ ## reg)
+	snd_hdac_reg_readb((dev)->sd_addr + AZX_REG_ ## reg)
 
 /* 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 f34aced69ca8..ef88b20c7b0a 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -6,7 +6,6 @@
 
 int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device *dev,
 		      const struct hdac_bus_ops *ops,
-		      const struct hdac_io_ops *io_ops,
 		      const struct hdac_ext_bus_ops *ext_ops);
 
 void snd_hdac_ext_bus_exit(struct hdac_bus *bus);
diff --git a/sound/hda/Kconfig b/sound/hda/Kconfig
index f6feced15f17..1d475cf3f754 100644
--- a/sound/hda/Kconfig
+++ b/sound/hda/Kconfig
@@ -6,6 +6,9 @@ config SND_HDA_CORE
 config SND_HDA_DSP_LOADER
 	bool
 
+config SND_HDA_ALIGNED_MMIO
+	bool
+
 config SND_HDA_COMPONENT
 	bool
 
diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
index 7825b74068f4..242306d820ec 100644
--- a/sound/hda/ext/hdac_ext_bus.c
+++ b/sound/hda/ext/hdac_ext_bus.c
@@ -17,67 +17,22 @@
 MODULE_DESCRIPTION("HDA extended core");
 MODULE_LICENSE("GPL v2");
 
-static void hdac_ext_writel(u32 value, u32 __iomem *addr)
-{
-	writel(value, addr);
-}
-
-static u32 hdac_ext_readl(u32 __iomem *addr)
-{
-	return readl(addr);
-}
-
-static void hdac_ext_writew(u16 value, u16 __iomem *addr)
-{
-	writew(value, addr);
-}
-
-static u16 hdac_ext_readw(u16 __iomem *addr)
-{
-	return readw(addr);
-}
-
-static void hdac_ext_writeb(u8 value, u8 __iomem *addr)
-{
-	writeb(value, addr);
-}
-
-static u8 hdac_ext_readb(u8 __iomem *addr)
-{
-	return readb(addr);
-}
-
-static const struct hdac_io_ops hdac_ext_default_io = {
-	.reg_writel = hdac_ext_writel,
-	.reg_readl = hdac_ext_readl,
-	.reg_writew = hdac_ext_writew,
-	.reg_readw = hdac_ext_readw,
-	.reg_writeb = hdac_ext_writeb,
-	.reg_readb = hdac_ext_readb,
-};
-
 /**
  * snd_hdac_ext_bus_init - initialize a HD-audio extended bus
  * @ebus: the pointer to extended bus object
  * @dev: device pointer
  * @ops: bus verb operators
- * @io_ops: lowlevel I/O operators, can be NULL. If NULL core will use
  * default ops
  *
  * Returns 0 if successful, or a negative error code.
  */
 int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device *dev,
 			const struct hdac_bus_ops *ops,
-			const struct hdac_io_ops *io_ops,
 			const struct hdac_ext_bus_ops *ext_ops)
 {
 	int ret;
 
-	/* check if io ops are provided, if not load the defaults */
-	if (io_ops == NULL)
-		io_ops = &hdac_ext_default_io;
-
-	ret = snd_hdac_bus_init(bus, dev, ops, io_ops);
+	ret = snd_hdac_bus_init(bus, dev, ops);
 	if (ret < 0)
 		return ret;
 
diff --git a/sound/hda/hdac_bus.c b/sound/hda/hdac_bus.c
index 00ea12e67dc8..dc2523ef7d98 100644
--- a/sound/hda/hdac_bus.c
+++ b/sound/hda/hdac_bus.c
@@ -19,13 +19,11 @@ static const struct hdac_bus_ops default_ops = {
  * snd_hdac_bus_init - initialize a HD-audio bas bus
  * @bus: the pointer to bus object
  * @ops: bus verb operators
- * @io_ops: lowlevel I/O operators
  *
  * Returns 0 if successful, or a negative error code.
  */
 int snd_hdac_bus_init(struct hdac_bus *bus, struct device *dev,
-		      const struct hdac_bus_ops *ops,
-		      const struct hdac_io_ops *io_ops)
+		      const struct hdac_bus_ops *ops)
 {
 	memset(bus, 0, sizeof(*bus));
 	bus->dev = dev;
@@ -33,7 +31,6 @@ int snd_hdac_bus_init(struct hdac_bus *bus, struct device *dev,
 		bus->ops = ops;
 	else
 		bus->ops = &default_ops;
-	bus->io_ops = io_ops;
 	bus->dma_type = SNDRV_DMA_TYPE_DEV;
 	INIT_LIST_HEAD(&bus->stream_list);
 	INIT_LIST_HEAD(&bus->codec_list);
@@ -218,3 +215,33 @@ void snd_hdac_bus_remove_device(struct hdac_bus *bus,
 	flush_work(&bus->unsol_work);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_remove_device);
+
+#ifdef CONFIG_SND_HDA_ALIGNED_MMIO
+/* Helpers for aligned read/write of mmio space, for Tegra */
+unsigned int snd_hdac_aligned_read(void __iomem *addr, unsigned int mask)
+{
+	void __iomem *aligned_addr =
+		(void __iomem *)((unsigned long)(addr) & ~0x3);
+	unsigned int shift = ((unsigned long)(addr) & 0x3) << 3;
+	unsigned int v;
+
+	v = readl(aligned_addr);
+	return (v >> shift) & mask;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_aligned_read);
+
+void snd_hdac_aligned_write(unsigned int val, void __iomem *addr,
+			    unsigned int mask)
+{
+	void __iomem *aligned_addr =
+		(void __iomem *)((unsigned long)(addr) & ~0x3);
+	unsigned int shift = ((unsigned long)(addr) & 0x3) << 3;
+	unsigned int v;
+
+	v = readl(aligned_addr);
+	v &= ~(mask << shift);
+	v |= val << shift;
+	writel(v, aligned_addr);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_aligned_write);
+#endif /* CONFIG_SND_HDA_ALIGNED_MMIO */
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 35d934309cb2..82198ea8f7f8 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -26,6 +26,7 @@ config SND_HDA_TEGRA
 	tristate "NVIDIA Tegra HD Audio"
 	depends on ARCH_TEGRA
 	select SND_HDA
+	select SND_HDA_ALIGNED_MMIO
 	help
 	  Say Y here to support the HDA controller present in NVIDIA
 	  Tegra SoCs
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index c8d1b4316245..ee5504e2441f 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -1202,14 +1202,12 @@ void snd_hda_bus_reset(struct hda_bus *bus)
 }
 
 /* HD-audio bus initialization */
-int azx_bus_init(struct azx *chip, const char *model,
-		 const struct hdac_io_ops *io_ops)
+int azx_bus_init(struct azx *chip, const char *model)
 {
 	struct hda_bus *bus = &chip->bus;
 	int err;
 
-	err = snd_hdac_bus_init(&bus->core, chip->card->dev, &bus_core_ops,
-				io_ops);
+	err = snd_hdac_bus_init(&bus->core, chip->card->dev, &bus_core_ops);
 	if (err < 0)
 		return err;
 
diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h
index baa15374fbcb..146a71e0d594 100644
--- a/sound/pci/hda/hda_controller.h
+++ b/sound/pci/hda/hda_controller.h
@@ -206,8 +206,7 @@ void azx_stop_chip(struct azx *chip);
 irqreturn_t azx_interrupt(int irq, void *dev_id);
 
 /* Codec interface */
-int azx_bus_init(struct azx *chip, const char *model,
-		 const struct hdac_io_ops *io_ops);
+int azx_bus_init(struct azx *chip, const char *model);
 int azx_probe_codecs(struct azx *chip, unsigned int max_slots);
 int azx_codec_configure(struct azx *chip);
 int azx_init_streams(struct azx *chip);
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 3bb4c26f2799..963a92943a6d 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1627,7 +1627,6 @@ static int default_bdl_pos_adj(struct azx *chip)
 /*
  * constructor
  */
-static const struct hdac_io_ops pci_hda_io_ops;
 static const struct hda_controller_ops pci_hda_ops;
 
 static int azx_create(struct snd_card *card, struct pci_dev *pci,
@@ -1687,7 +1686,7 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
 	else
 		chip->bdl_pos_adj = bdl_pos_adj[dev];
 
-	err = azx_bus_init(chip, model[dev], &pci_hda_io_ops);
+	err = azx_bus_init(chip, model[dev]);
 	if (err < 0) {
 		kfree(hda);
 		pci_disable_device(pci);
@@ -1932,41 +1931,6 @@ static void azx_firmware_cb(const struct firmware *fw, void *context)
 }
 #endif
 
-/*
- * HDA controller ops.
- */
-
-/* PCI register access. */
-static void pci_azx_writel(u32 value, u32 __iomem *addr)
-{
-	writel(value, addr);
-}
-
-static u32 pci_azx_readl(u32 __iomem *addr)
-{
-	return readl(addr);
-}
-
-static void pci_azx_writew(u16 value, u16 __iomem *addr)
-{
-	writew(value, addr);
-}
-
-static u16 pci_azx_readw(u16 __iomem *addr)
-{
-	return readw(addr);
-}
-
-static void pci_azx_writeb(u8 value, u8 __iomem *addr)
-{
-	writeb(value, addr);
-}
-
-static u8 pci_azx_readb(u8 __iomem *addr)
-{
-	return readb(addr);
-}
-
 static int disable_msi_reset_irq(struct azx *chip)
 {
 	struct hdac_bus *bus = azx_bus(chip);
@@ -1994,15 +1958,6 @@ static void pcm_mmap_prepare(struct snd_pcm_substream *substream,
 #endif
 }
 
-static const struct hdac_io_ops pci_hda_io_ops = {
-	.reg_writel = pci_azx_writel,
-	.reg_readl = pci_azx_readl,
-	.reg_writew = pci_azx_writew,
-	.reg_readw = pci_azx_readw,
-	.reg_writeb = pci_azx_writeb,
-	.reg_readb = pci_azx_readb,
-};
-
 static const struct hda_controller_ops pci_hda_ops = {
 	.disable_msi_reset_irq = disable_msi_reset_irq,
 	.pcm_mmap_prepare = pcm_mmap_prepare,
diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index ba414cc639f1..8350954b7986 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -75,72 +75,6 @@ MODULE_PARM_DESC(power_save,
 #define power_save	0
 #endif
 
-/*
- * Register access ops. Tegra HDA register access is DWORD only.
- */
-static void hda_tegra_writel(u32 value, u32 __iomem *addr)
-{
-	writel(value, addr);
-}
-
-static u32 hda_tegra_readl(u32 __iomem *addr)
-{
-	return readl(addr);
-}
-
-static void hda_tegra_writew(u16 value, u16 __iomem  *addr)
-{
-	unsigned int shift = ((unsigned long)(addr) & 0x3) << 3;
-	void __iomem *dword_addr = (void __iomem *)((unsigned long)(addr) & ~0x3);
-	u32 v;
-
-	v = readl(dword_addr);
-	v &= ~(0xffff << shift);
-	v |= value << shift;
-	writel(v, dword_addr);
-}
-
-static u16 hda_tegra_readw(u16 __iomem *addr)
-{
-	unsigned int shift = ((unsigned long)(addr) & 0x3) << 3;
-	void __iomem *dword_addr = (void __iomem *)((unsigned long)(addr) & ~0x3);
-	u32 v;
-
-	v = readl(dword_addr);
-	return (v >> shift) & 0xffff;
-}
-
-static void hda_tegra_writeb(u8 value, u8 __iomem *addr)
-{
-	unsigned int shift = ((unsigned long)(addr) & 0x3) << 3;
-	void __iomem *dword_addr = (void __iomem *)((unsigned long)(addr) & ~0x3);
-	u32 v;
-
-	v = readl(dword_addr);
-	v &= ~(0xff << shift);
-	v |= value << shift;
-	writel(v, dword_addr);
-}
-
-static u8 hda_tegra_readb(u8 __iomem *addr)
-{
-	unsigned int shift = ((unsigned long)(addr) & 0x3) << 3;
-	void __iomem *dword_addr = (void __iomem *)((unsigned long)(addr) & ~0x3);
-	u32 v;
-
-	v = readl(dword_addr);
-	return (v >> shift) & 0xff;
-}
-
-static const struct hdac_io_ops hda_tegra_io_ops = {
-	.reg_writel = hda_tegra_writel,
-	.reg_readl = hda_tegra_readl,
-	.reg_writew = hda_tegra_writew,
-	.reg_readw = hda_tegra_readw,
-	.reg_writeb = hda_tegra_writeb,
-	.reg_readb = hda_tegra_readb,
-};
-
 static const struct hda_controller_ops hda_tegra_ops; /* nothing special */
 
 static void hda_tegra_init(struct hda_tegra *hda)
@@ -459,7 +393,7 @@ static int hda_tegra_create(struct snd_card *card,
 
 	INIT_WORK(&hda->probe_work, hda_tegra_probe_work);
 
-	err = azx_bus_init(chip, NULL, &hda_tegra_io_ops);
+	err = azx_bus_init(chip, NULL);
 	if (err < 0)
 		return err;
 
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 3362e71b4563..c6d8076dc2fd 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -132,7 +132,7 @@ static int skl_init_chip(struct hdac_bus *bus, bool full_reset)
 
 	/* Reset stream-to-link mapping */
 	list_for_each_entry(hlink, &bus->hlink_list, list)
-		bus->io_ops->reg_writel(0, hlink->ml_addr + AZX_REG_ML_LOSIDV);
+		writel(0, hlink->ml_addr + AZX_REG_ML_LOSIDV);
 
 	skl_enable_miscbdcge(bus->dev, true);
 
@@ -854,7 +854,6 @@ static void skl_probe_work(struct work_struct *work)
  * constructor
  */
 static int skl_create(struct pci_dev *pci,
-		      const struct hdac_io_ops *io_ops,
 		      struct skl **rskl)
 {
 	struct hdac_ext_bus_ops *ext_ops = NULL;
@@ -884,7 +883,7 @@ static int skl_create(struct pci_dev *pci,
 #if IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
 	ext_ops = snd_soc_hdac_hda_get_ops();
 #endif
-	snd_hdac_ext_bus_init(bus, &pci->dev, &bus_core_ops, io_ops, ext_ops);
+	snd_hdac_ext_bus_init(bus, &pci->dev, &bus_core_ops, ext_ops);
 	bus->use_posbuf = 1;
 	skl->pci = pci;
 	INIT_WORK(&skl->probe_work, skl_probe_work);
@@ -1013,7 +1012,7 @@ static int skl_probe(struct pci_dev *pci,
 	}
 
 	/* we use ext core ops, so provide NULL for ops here */
-	err = skl_create(pci, NULL, &skl);
+	err = skl_create(pci, &skl);
 	if (err < 0)
 		return err;
 
diff --git a/sound/soc/sof/intel/hda-bus.c b/sound/soc/sof/intel/hda-bus.c
index 0bc93fa06b5b..438121c70f99 100644
--- a/sound/soc/sof/intel/hda-bus.c
+++ b/sound/soc/sof/intel/hda-bus.c
@@ -21,45 +21,6 @@ static const struct hdac_bus_ops bus_ops = {
 
 #endif
 
-static void sof_hda_writel(u32 value, u32 __iomem *addr)
-{
-	writel(value, addr);
-}
-
-static u32 sof_hda_readl(u32 __iomem *addr)
-{
-	return readl(addr);
-}
-
-static void sof_hda_writew(u16 value, u16 __iomem *addr)
-{
-	writew(value, addr);
-}
-
-static u16 sof_hda_readw(u16 __iomem *addr)
-{
-	return readw(addr);
-}
-
-static void sof_hda_writeb(u8 value, u8 __iomem *addr)
-{
-	writeb(value, addr);
-}
-
-static u8 sof_hda_readb(u8 __iomem *addr)
-{
-	return readb(addr);
-}
-
-static const struct hdac_io_ops io_ops = {
-	.reg_writel = sof_hda_writel,
-	.reg_readl = sof_hda_readl,
-	.reg_writew = sof_hda_writew,
-	.reg_readw = sof_hda_readw,
-	.reg_writeb = sof_hda_writeb,
-	.reg_readb = sof_hda_readb,
-};
-
 /*
  * This can be used for both with/without hda link support.
  */
@@ -69,7 +30,6 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev,
 	memset(bus, 0, sizeof(*bus));
 	bus->dev = dev;
 
-	bus->io_ops = &io_ops;
 	INIT_LIST_HEAD(&bus->stream_list);
 
 	bus->irq = -1;
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 91de4785b6a3..8d4ce5b4febd 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -356,7 +356,7 @@ static int hda_resume(struct snd_sof_dev *sdev)
 
 	/* Reset stream-to-link mapping */
 	list_for_each_entry(hlink, &bus->hlink_list, list)
-		bus->io_ops->reg_writel(0, hlink->ml_addr + AZX_REG_ML_LOSIDV);
+		writel(0, hlink->ml_addr + AZX_REG_ML_LOSIDV);
 
 	hda_dsp_ctrl_misc_clock_gating(sdev, true);
 #else
-- 
2.16.4

[PATCH 3/3] ASoC: SOF: Intel: Initialize hdaudio bus properly

[Takashi Iwai]

The SOF HD-audio bus has its house-made initialization code.  It's
supposedly for making the code independent from HD-audio bus drivers.
However, this is error-prone, and above all, the SOF driver has
already dependency on HD-audio bus driver when CONFIG_SND_SOF_HDA is
set.  That is, if this Kconfig is set, there is no reason to avoid the
call to the proper bus init function.

Also, the ext_ops that is set at bus initialization can be better
handled inside sof_hda_bus_init().  We don't need to refer this
outside the bus initialization.

So this patch addresses these issues:
- sof_hda_bus_init() calls nothing but snd_hdac_ext_bus_init()
  when CONFIG_SND_SOF_HDA is set.  Otherwise some fields are
  initialized locally like before for avoiding the dependency.
- ext_ops is referred inside sof_hda_bus_init().  The ext_ops argument
  of snd_hda_bus_init() is dropped.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/soc/sof/intel/hda-bus.c | 31 +++++++++----------------------
 sound/soc/sof/intel/hda.c     |  6 +-----
 sound/soc/sof/intel/hda.h     |  3 +--
 3 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/sound/soc/sof/intel/hda-bus.c b/sound/soc/sof/intel/hda-bus.c
index 438121c70f99..0caec3a070d3 100644
--- a/sound/soc/sof/intel/hda-bus.c
+++ b/sound/soc/sof/intel/hda-bus.c
@@ -12,28 +12,26 @@
 #include "../sof-priv.h"
 #include "hda.h"
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
-
-static const struct hdac_bus_ops bus_ops = {
-	.command = snd_hdac_bus_send_cmd,
-	.get_response = snd_hdac_bus_get_response,
-};
-
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
+#define sof_hda_ext_ops	snd_soc_hdac_hda_get_ops()
+#else
+#define sof_hda_ext_ops	NULL
 #endif
 
 /*
  * This can be used for both with/without hda link support.
  */
-void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev,
-		      const struct hdac_ext_bus_ops *ext_ops)
+void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev)
 {
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
+	snd_hdac_ext_bus_init(bus, dev, NULL, sof_hda_ext_ops);
+#else /* CONFIG_SND_SOC_SOF_HDA */
 	memset(bus, 0, sizeof(*bus));
 	bus->dev = dev;
 
 	INIT_LIST_HEAD(&bus->stream_list);
 
 	bus->irq = -1;
-	bus->ext_ops = ext_ops;
 
 	/*
 	 * There is only one HDA bus atm. keep the index as 0.
@@ -42,16 +40,5 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev,
 	bus->idx = 0;
 
 	spin_lock_init(&bus->reg_lock);
-
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
-	INIT_LIST_HEAD(&bus->codec_list);
-	INIT_LIST_HEAD(&bus->hlink_list);
-
-	mutex_init(&bus->cmd_mutex);
-	mutex_init(&bus->lock);
-	bus->ops = &bus_ops;
-	INIT_WORK(&bus->unsol_work, snd_hdac_bus_process_unsol_events);
-	bus->cmd_dma_state = true;
-#endif
-
+#endif /* CONFIG_SND_SOC_SOF_HDA */
 }
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 7f665392618f..7ca27000c34d 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -236,7 +236,6 @@ static int hda_init(struct snd_sof_dev *sdev)
 {
 	struct hda_bus *hbus;
 	struct hdac_bus *bus;
-	struct hdac_ext_bus_ops *ext_ops = NULL;
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
 	int ret;
 
@@ -244,10 +243,7 @@ static int hda_init(struct snd_sof_dev *sdev)
 	bus = sof_to_bus(sdev);
 
 	/* HDA bus init */
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
-	ext_ops = snd_soc_hdac_hda_get_ops();
-#endif
-	sof_hda_bus_init(bus, &pci->dev, ext_ops);
+	sof_hda_bus_init(bus, &pci->dev);
 
 	/* Workaround for a communication error on CFL (bko#199007) and CNL */
 	if (IS_CFL(pci) || IS_CNL(pci))
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index d9c17146200b..75b096050fa2 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -549,8 +549,7 @@ void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev);
 /*
  * HDA bus operations.
  */
-void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev,
-		      const struct hdac_ext_bus_ops *ext_ops);
+void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev);
 
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
 /*
-- 
2.16.4

Re: [PATCH 3/3] ASoC: SOF: Intel: Initialize hdaudio bus properly

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 496 bytes --]

On Thu, Aug 08, 2019 at 11:57:15AM +0200, Takashi Iwai wrote:
> The SOF HD-audio bus has its house-made initialization code.  It's
> supposedly for making the code independent from HD-audio bus drivers.
> However, this is error-prone, and above all, the SOF driver has
> already dependency on HD-audio bus driver when CONFIG_SND_SOF_HDA is
> set.  That is, if this Kconfig is set, there is no reason to avoid the
> call to the proper bus init function.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/3] ALSA: hda: bus cleanup

[Pierre-Louis Bossart]

On 8/8/19 4:57 AM, Takashi Iwai wrote:
> Hi,
> 
> this is a few patches to simplify and cleanup the HD-audio bus ops.
> 
> The first two patches translate the indirect calls of DMA page
> allocation and MMIO accesses with the direct ones, as well as
> eliminating the whole bus->io_ops.
> 
> The last one is SOF-specific, and fixes/cleans up by calling the
> proper hdaudio bus init function, as formerly discussed.

This is a good cleanup, thanks Takashi.

For the series

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

One request from me since I am lazy: could we somehow merge branches 
between you / Mark since at the moment we have two sets of conflicts 
(NHLT and this one). We test all the upstream changes on hardware as 
soon as we can, and manual conflict resolution makes it slower/more 
error prone.

> 
> 
> Takashi
> 
> ===
> 
> Takashi Iwai (3):
>    ALSA: hda: Remove page allocation redirection
>    ALSA: hda: Direct MMIO accesses
>    ASoC: SOF: Intel: Initialize hdaudio bus properly
> 
>   include/sound/hdaudio.h                | 69 +++++++++++++--------------
>   include/sound/hdaudio_ext.h            |  1 -
>   sound/hda/Kconfig                      |  3 ++
>   sound/hda/ext/hdac_ext_bus.c           | 60 +-----------------------
>   sound/hda/hdac_bus.c                   | 36 ++++++++++++--
>   sound/hda/hdac_controller.c            | 18 +++----
>   sound/hda/hdac_stream.c                |  8 ++--
>   sound/pci/hda/Kconfig                  |  1 +
>   sound/pci/hda/hda_controller.c         |  6 +--
>   sound/pci/hda/hda_controller.h         |  3 +-
>   sound/pci/hda/hda_intel.c              | 71 ++--------------------------
>   sound/pci/hda/hda_tegra.c              | 84 +--------------------------------
>   sound/soc/intel/skylake/skl-messages.c | 15 +-----
>   sound/soc/intel/skylake/skl.c          |  7 ++-
>   sound/soc/sof/intel/hda-bus.c          | 85 ++++------------------------------
>   sound/soc/sof/intel/hda-dsp.c          |  2 +-
>   sound/soc/sof/intel/hda.c              |  6 +--
>   sound/soc/sof/intel/hda.h              |  3 +-
>   18 files changed, 107 insertions(+), 371 deletions(-)
> 

Re: [PATCH 0/3] ALSA: hda: bus cleanup

[Takashi Iwai]

On Thu, 08 Aug 2019 16:23:40 +0200,
Pierre-Louis Bossart wrote:
> 
> One request from me since I am lazy: could we somehow merge branches
> between you / Mark since at the moment we have two sets of conflicts
> (NHLT and this one). We test all the upstream changes on hardware as
> soon as we can, and manual conflict resolution makes it slower/more
> error prone.

I don't mind who actually merges.  I'm going to push out my topic
branch once after gathering enough Ack's.  Then Mark can pull it into
his tree (as well as topic/hda-dmic branch).

Or if Mark can give me the branch, I can pull into my for-next
branch, too.  Basically my for-next branch is persistent and not
rebased, so you can work on it once when set.

Mark, just let me know your preference.


thanks,

Takashi

Re: [PATCH 0/3] ALSA: hda: bus cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 673 bytes --]

On Thu, Aug 08, 2019 at 04:45:09PM +0200, Takashi Iwai wrote:

> I don't mind who actually merges.  I'm going to push out my topic
> branch once after gathering enough Ack's.  Then Mark can pull it into
> his tree (as well as topic/hda-dmic branch).

I can do that easily enough.

> Or if Mark can give me the branch, I can pull into my for-next
> branch, too.  Basically my for-next branch is persistent and not
> rebased, so you can work on it once when set.

> Mark, just let me know your preference.

Well, I don't have topic branches any more since Linus hates them and
it's always hard to write sensible summaries for the development when
it's half way through so...

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/3] ALSA: hda: bus cleanup

[Takashi Iwai]

On Thu, 08 Aug 2019 21:03:43 +0200,
Mark Brown wrote:
> 
> On Thu, Aug 08, 2019 at 04:45:09PM +0200, Takashi Iwai wrote:
> 
> > I don't mind who actually merges.  I'm going to push out my topic
> > branch once after gathering enough Ack's.  Then Mark can pull it into
> > his tree (as well as topic/hda-dmic branch).
> 
> I can do that easily enough.

OK, I pushed out the branch topic/hda-bus-ops-cleanup.
Please merge this one and topic/hda-dmic branch into yours.


thanks,

Takashi

Re: [PATCH 0/3] ALSA: hda: bus cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 352 bytes --]

On Thu, Aug 08, 2019 at 10:38:33PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > I can do that easily enough.

> OK, I pushed out the branch topic/hda-bus-ops-cleanup.
> Please merge this one and topic/hda-dmic branch into yours.

There were some conflicts which hopefully I resolved OK, it compiles and
everything so it can't be that bad right?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/3] ALSA: hda: bus cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 598 bytes --]

On Thu, Aug 08, 2019 at 10:33:11PM +0100, Mark Brown wrote:
> On Thu, Aug 08, 2019 at 10:38:33PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > I can do that easily enough.
> 
> > OK, I pushed out the branch topic/hda-bus-ops-cleanup.
> > Please merge this one and topic/hda-dmic branch into yours.
> 
> There were some conflicts which hopefully I resolved OK, it compiles and
> everything so it can't be that bad right?

Had to rebuild this since git has a bug with recording merge conflicts,
just pushing out a new version now - c2f16a94a80497e4b28c27f9ca2cd6cd60706fb6.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/3] ALSA: hda: bus cleanup

[Takashi Iwai]

On Fri, 09 Aug 2019 00:22:01 +0200,
Mark Brown wrote:
> 
> On Thu, Aug 08, 2019 at 10:33:11PM +0100, Mark Brown wrote:
> > On Thu, Aug 08, 2019 at 10:38:33PM +0200, Takashi Iwai wrote:
> > > Mark Brown wrote:
> > 
> > > > I can do that easily enough.
> > 
> > > OK, I pushed out the branch topic/hda-bus-ops-cleanup.
> > > Please merge this one and topic/hda-dmic branch into yours.
> > 
> > There were some conflicts which hopefully I resolved OK, it compiles and
> > everything so it can't be that bad right?
> 
> Had to rebuild this since git has a bug with recording merge conflicts,
> just pushing out a new version now - c2f16a94a80497e4b28c27f9ca2cd6cd60706fb6.

Thanks, it looks good to me.

And Stephen resolved the conflicts in today's linux-next, too, so
let's compare later.


Takashi

Re: [PATCH 0/3] ALSA: hda: bus cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 457 bytes --]

On Fri, Aug 09, 2019 at 07:58:06AM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > Had to rebuild this since git has a bug with recording merge conflicts,
> > just pushing out a new version now - c2f16a94a80497e4b28c27f9ca2cd6cd60706fb6.

> Thanks, it looks good to me.

> And Stephen resolved the conflicts in today's linux-next, too, so
> let's compare later.

My tree didn't get updated in -next because there's some configurations
that don't build.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]