[alsa-devel] Why open-coding in sof_hda_bus_init()?

[Takashi Iwai]

Hi,

while looking at SOF code due to the recent debugging session, I
noticed that sof_hda_bus_init() is basically an open-code of the
existing snd_hdac_ext_bus_init().  Why don't we simply call
snd_hdac_ext_bus_init() like below?

The only thing that differs is the handling of bus->id number, where
SOF fixes to zero (which was actually done in a patch after the first
commit).  But judging from the comment, this doesn't seem like a big
matter, as we assume a single bus, so far...


thanks,

Takashi

---
diff --git a/sound/soc/sof/intel/Makefile b/sound/soc/sof/intel/Makefile
index b8f58e006e29..f42450a9a7b6 100644
--- a/sound/soc/sof/intel/Makefile
+++ b/sound/soc/sof/intel/Makefile
@@ -7,7 +7,7 @@ snd-sof-intel-ipc-objs := intel-ipc.o
 
 snd-sof-intel-hda-common-objs := hda.o hda-loader.o hda-stream.o hda-trace.o \
 				 hda-dsp.o hda-ipc.o hda-ctrl.o hda-pcm.o \
-				 hda-dai.o hda-bus.o \
+				 hda-dai.o \
 				 apl.o cnl.o
 
 snd-sof-intel-hda-objs := hda-codec.o
diff --git a/sound/soc/sof/intel/hda-bus.c b/sound/soc/sof/intel/hda-bus.c
deleted file mode 100644
index a7e6d8227df6..000000000000
--- a/sound/soc/sof/intel/hda-bus.c
+++ /dev/null
@@ -1,111 +0,0 @@
-// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
-//
-// This file is provided under a dual BSD/GPLv2 license.  When using or
-// redistributing this file, you may do so under either license.
-//
-// Copyright(c) 2018 Intel Corporation. All rights reserved.
-//
-// Authors: Keyon Jie <yang.jie at linux.intel.com>
-
-#include <linux/io.h>
-#include <sound/hdaudio.h>
-#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,
-};
-
-#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 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,
-	.reg_writew = sof_hda_writew,
-	.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,
-};
-
-/*
- * 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)
-{
-	memset(bus, 0, sizeof(*bus));
-	bus->dev = dev;
-
-	bus->io_ops = &io_ops;
-	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.
-	 * Need to fix when there are more than one HDA bus.
-	 */
-	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
-
-}
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 92d45c43b4b1..a9de6a297b56 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -532,8 +532,11 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset);
 /*
  * HDA bus operations.
  */
-void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev,
-		      const struct hdac_ext_bus_ops *ext_ops);
+static inline void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev,
+				    const struct hdac_ext_bus_ops *ext_ops)
+{
+	snd_hdac_ext_bus_init(bus, dev, NULL, NULL, ext_ops);
+}
 
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
 /*