All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org, Keyon Jie <yang.jie@linux.intel.com>
Subject: Why open-coding in sof_hda_bus_init()?
Date: Fri, 31 May 2019 19:11:57 +0200	[thread overview]
Message-ID: <s5h7ea6lfaa.wl-tiwai@suse.de> (raw)

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@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)
 /*

             reply	other threads:[~2019-05-31 17:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-31 17:11 Takashi Iwai [this message]
2019-05-31 17:43 ` Why open-coding in sof_hda_bus_init()? Pierre-Louis Bossart
2019-05-31 18:22   ` Takashi Iwai
2019-05-31 18:31     ` Pierre-Louis Bossart
2019-05-31 19:06       ` Takashi Iwai
2019-05-31 19:44         ` Takashi Iwai
2019-05-31 20:23           ` Pierre-Louis Bossart
2019-05-31 20:36             ` Takashi Iwai
2019-05-31 20:59               ` Pierre-Louis Bossart
2019-05-31 21:20                 ` Takashi Iwai
2019-06-03  5:58                   ` Keyon Jie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5h7ea6lfaa.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=yang.jie@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.