diff mbox

[v3,1/7] ASoC: hda - add soc hda codec driver wrapper

Message ID 1430250870-3169-2-git-send-email-vinod.koul@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vinod Koul April 28, 2015, 7:54 p.m. UTC
From: Jeeja KP <jeeja.kp@intel.com>

For ASoC HDA codecs we need to provide match function based on id_table and
driver register/unregister wrapper functions

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/Kconfig             |    1 +
 sound/soc/Makefile            |    1 +
 sound/soc/hda/Kconfig         |    3 ++
 sound/soc/hda/Makefile        |    3 ++
 sound/soc/hda/soc-hda-codec.c |   85 +++++++++++++++++++++++++++++++++++++++++
 sound/soc/hda/soc-hda-codec.h |   44 +++++++++++++++++++++
 6 files changed, 137 insertions(+)
 create mode 100644 sound/soc/hda/Kconfig
 create mode 100644 sound/soc/hda/Makefile
 create mode 100644 sound/soc/hda/soc-hda-codec.c
 create mode 100644 sound/soc/hda/soc-hda-codec.h

Comments

Takashi Iwai April 29, 2015, 11:59 a.m. UTC | #1
At Wed, 29 Apr 2015 01:24:24 +0530,
Vinod Koul wrote:
> 
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> For ASoC HDA codecs we need to provide match function based on id_table and
> driver register/unregister wrapper functions
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/soc/Kconfig             |    1 +
>  sound/soc/Makefile            |    1 +
>  sound/soc/hda/Kconfig         |    3 ++
>  sound/soc/hda/Makefile        |    3 ++
>  sound/soc/hda/soc-hda-codec.c |   85 +++++++++++++++++++++++++++++++++++++++++
>  sound/soc/hda/soc-hda-codec.h |   44 +++++++++++++++++++++
>  6 files changed, 137 insertions(+)
>  create mode 100644 sound/soc/hda/Kconfig
>  create mode 100644 sound/soc/hda/Makefile
>  create mode 100644 sound/soc/hda/soc-hda-codec.c
>  create mode 100644 sound/soc/hda/soc-hda-codec.h
> 
> diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
> index 3ba52da18bc6..d3903580cb10 100644
> --- a/sound/soc/Kconfig
> +++ b/sound/soc/Kconfig
> @@ -40,6 +40,7 @@ source "sound/soc/cirrus/Kconfig"
>  source "sound/soc/davinci/Kconfig"
>  source "sound/soc/dwc/Kconfig"
>  source "sound/soc/fsl/Kconfig"
> +source "sound/soc/hda/Kconfig"
>  source "sound/soc/jz4740/Kconfig"
>  source "sound/soc/nuc900/Kconfig"
>  source "sound/soc/omap/Kconfig"
> diff --git a/sound/soc/Makefile b/sound/soc/Makefile
> index 974ba708b482..8741d6a38bf6 100644
> --- a/sound/soc/Makefile
> +++ b/sound/soc/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_SND_SOC)	+= cirrus/
>  obj-$(CONFIG_SND_SOC)	+= davinci/
>  obj-$(CONFIG_SND_SOC)	+= dwc/
>  obj-$(CONFIG_SND_SOC)	+= fsl/
> +obj-$(CONFIG_SND_SOC)	+= hda/
>  obj-$(CONFIG_SND_SOC)	+= jz4740/
>  obj-$(CONFIG_SND_SOC)	+= intel/
>  obj-$(CONFIG_SND_SOC)	+= mxs/
> diff --git a/sound/soc/hda/Kconfig b/sound/soc/hda/Kconfig
> new file mode 100644
> index 000000000000..815943360bc5
> --- /dev/null
> +++ b/sound/soc/hda/Kconfig
> @@ -0,0 +1,3 @@
> +config SND_SOC_HDA_CORE
> +	tristate
> +	select SND_HDA_CORE
> diff --git a/sound/soc/hda/Makefile b/sound/soc/hda/Makefile
> new file mode 100644
> index 000000000000..9585ab180a55
> --- /dev/null
> +++ b/sound/soc/hda/Makefile
> @@ -0,0 +1,3 @@
> +snd-soc-hda-core-objs := soc-hda-codec.o
> +
> +obj-$(CONFIG_SND_SOC_HDA_CORE) += snd-soc-hda-core.o
> diff --git a/sound/soc/hda/soc-hda-codec.c b/sound/soc/hda/soc-hda-codec.c
> new file mode 100644
> index 000000000000..caceb2136bba
> --- /dev/null
> +++ b/sound/soc/hda/soc-hda-codec.c
> @@ -0,0 +1,85 @@
> +/*
> + * soc-hda-codec.c ASoC High Definition Audio Codec interface Definitions
> + *
> + * Copyright (c) 2014-2015 Intel Corporation
> + *
> + * Author(s): Jeeja KP <jeeja.kp@intel.com>
> + *
> + *
> + *  This driver is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This driver is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <sound/hdaudio.h>
> +#include "soc-hda-codec.h"
> +
> +/**
> + * snd_soc_hda_get_device_id - gets the hdac device id entry
> + * @hdev: HD-audio core bus

Not a bus.

> + * @drv: HD-audio core stream object to initialize

Not a stream.

> + * Compares the hdac device vendor_id and revision_id to the hdac_soc_codec
> + * driver id_table and returns the matching device id entry.
> + */
> +const struct soc_hda_device_id *
> +snd_soc_hda_get_device_id(
> +			struct hdac_device *hdev,
> +			struct soc_hda_codec_driver *drv)

The indentation looks a bid odd.

> diff --git a/sound/soc/hda/soc-hda-codec.h b/sound/soc/hda/soc-hda-codec.h
> new file mode 100644
> index 000000000000..f8a199a58926
> --- /dev/null
> +++ b/sound/soc/hda/soc-hda-codec.h
> @@ -0,0 +1,44 @@
> +/*
> + * ASoC High Definition Audio Codec interface
> + *
> + * Copyright (c) 2014-2015 Intel Corporation
> + *
> + * Author(s): Jeeja KP <jeeja.kp@intel.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify it
> + *  under the terms of the GNU General Public License as published by the Free
> + *  Software Foundation; either version 2 of the License, or (at your option)
> + *  any later version.
> + *
> + *  This program is distributed in the hope that it will be useful, but WITHOUT
> + *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + *  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + *  more details.
> + *
> + */
> +
> +#include <sound/hdaudio.h>
> +
> +/*HDA device table*/
> +#define HDA_NAME_SIZE      20
> +struct soc_hda_device_id {
> +	__u32 id;
> +	__u32 rev_id;
> +	char name[HDA_NAME_SIZE];
> +	unsigned long driver_data;
> +};
> +
> +struct soc_hda_codec_driver {
> +	struct hdac_driver core;
> +	const struct soc_hda_device_id *id_table;
> +};
> +
> +#define to_hdac_driver(drv) (container_of((drv), \
> +				 struct hdac_driver, driver))
> +#define to_soc_hda_codec_driver(hdrv) (container_of((hdrv), \
> +				 struct soc_hda_codec_driver, core))
> +
> +int snd_soc_hda_codec_driver_register(struct soc_hda_codec_driver *drv);
> +void snd_soc_hda_codec_driver_unregister(struct soc_hda_codec_driver *drv);
> +const struct soc_hda_device_id *snd_soc_hda_get_device_id(
> +	struct hdac_device *hdev, struct soc_hda_codec_driver *drv);

The ifdef guard is missing.


Takashi
Mark Brown May 4, 2015, 1:12 p.m. UTC | #2
On Wed, Apr 29, 2015 at 01:24:24AM +0530, Vinod Koul wrote:

> For ASoC HDA codecs we need to provide match function based on id_table and
> driver register/unregister wrapper functions

This changelog doesn't really leave me that much the wiser as to what
this is intended to do...  what are we matching in what ID table and
what are we wrapping?

> ---
>  sound/soc/Kconfig             |    1 +
>  sound/soc/Makefile            |    1 +
>  sound/soc/hda/Kconfig         |    3 ++
>  sound/soc/hda/Makefile        |    3 ++
>  sound/soc/hda/soc-hda-codec.c |   85 +++++++++++++++++++++++++++++++++++++++++
>  sound/soc/hda/soc-hda-codec.h |   44 +++++++++++++++++++++
>  6 files changed, 137 insertions(+)

If this is for CODECs why is it in a new directory?

> +const struct soc_hda_device_id *
> +snd_soc_hda_get_device_id(
> +			struct hdac_device *hdev,
> +			struct soc_hda_codec_driver *drv)

Please can we have more normal indentation - put at least the first
argument for the function on the same line as the function name.
Vinod Koul May 6, 2015, 3:47 a.m. UTC | #3
On Mon, May 04, 2015 at 02:12:26PM +0100, Mark Brown wrote:
> On Wed, Apr 29, 2015 at 01:24:24AM +0530, Vinod Koul wrote:
> 
> > For ASoC HDA codecs we need to provide match function based on id_table and
> > driver register/unregister wrapper functions
> 
> This changelog doesn't really leave me that much the wiser as to what
> this is intended to do...  what are we matching in what ID table and
> what are we wrapping?
> 
> > ---
> >  sound/soc/Kconfig             |    1 +
> >  sound/soc/Makefile            |    1 +
> >  sound/soc/hda/Kconfig         |    3 ++
> >  sound/soc/hda/Makefile        |    3 ++
> >  sound/soc/hda/soc-hda-codec.c |   85 +++++++++++++++++++++++++++++++++++++++++
> >  sound/soc/hda/soc-hda-codec.h |   44 +++++++++++++++++++++
> >  6 files changed, 137 insertions(+)
> 
> If this is for CODECs why is it in a new directory?
yes it is for HDA codecs which are enumerated over HDA links, but this is
the HDA bus code which is ASoC specfic. All generic HDA code which uses the
hda lib (new sound/hda) is kept in sound/soc/hda
All intel driver updates will go to sound/soc/intel/skylake
The codecs drivers will as usual show up in sound/soc/codecs/

> 
> > +const struct soc_hda_device_id * +snd_soc_hda_get_device_id( +
> > struct hdac_device *hdev, +			struct soc_hda_codec_driver
> > *drv)
> 
> Please can we have more normal indentation - put at least the first
> argument for the function on the same line as the function name.
Yes Takashi also pointed out, fixed now. The zeal to keep under 80 chars
made it look funny.

Thanks
Mark Brown May 6, 2015, 12:51 p.m. UTC | #4
On Wed, May 06, 2015 at 09:17:50AM +0530, Vinod Koul wrote:
> On Mon, May 04, 2015 at 02:12:26PM +0100, Mark Brown wrote:

> > If this is for CODECs why is it in a new directory?

> yes it is for HDA codecs which are enumerated over HDA links, but this is
> the HDA bus code which is ASoC specfic. All generic HDA code which uses the
> hda lib (new sound/hda) is kept in sound/soc/hda
> All intel driver updates will go to sound/soc/intel/skylake
> The codecs drivers will as usual show up in sound/soc/codecs/

I'm still not 100% clear as to what a "HDA CODEC wrapper" is intended to
be.  I think this series probably needs an introduction which describes
what the overall picture we're aiming for is.
Vinod Koul May 6, 2015, 4:51 p.m. UTC | #5
On Wed, May 06, 2015 at 01:51:23PM +0100, Mark Brown wrote:
> On Wed, May 06, 2015 at 09:17:50AM +0530, Vinod Koul wrote:
> > On Mon, May 04, 2015 at 02:12:26PM +0100, Mark Brown wrote:
> 
> > > If this is for CODECs why is it in a new directory?
> 
> > yes it is for HDA codecs which are enumerated over HDA links, but this is
> > the HDA bus code which is ASoC specfic. All generic HDA code which uses the
> > hda lib (new sound/hda) is kept in sound/soc/hda
> > All intel driver updates will go to sound/soc/intel/skylake
> > The codecs drivers will as usual show up in sound/soc/codecs/
> 
> I'm still not 100% clear as to what a "HDA CODEC wrapper" is intended to
> be.  I think this series probably needs an introduction which describes
> what the overall picture we're aiming for is.
okay let me try that here as well as next version which i will post
tomorrow.

So SKL has HDA controller based audio subsystem with DSP and gets better
with support for I2S, HDA, PDM links.

So we worked with Takashi on HDA parts and now we have core code of HDA moved
into sound/hda which current HDA drivers use and will also be used by ASoC
SKL driver.

The SKL platform driver will load and create the soc_hdac_bus which would
embed the hdac_bus, same for hdac_device (hda codecs) and hdac_stream (pcms)
This is on top of hdac code in Takashi's topic/hda

This patch provides the match function for asoc type hda codecs and lets
them get enumerated by hdac. The second patch in this series adds the
controller specific soc code. Common parts are in hdac core with changes
introduced as part of SKL controller in soc part. Then we add the rest of
controller PCM driver code (still HDA) and last patch breaks the HDA streams
to host and link which will allow insertion of DSP in between these links.

The subsequent series will add IPC driver for SKL (using common IPC
routines), then DSP topology handlers, DSP code with I2S support and then
lastly when DFW is accepted then its handlers.

Let me know if you have more questions around this, would be happy to answer
them :)

Thanks
diff mbox

Patch

diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 3ba52da18bc6..d3903580cb10 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -40,6 +40,7 @@  source "sound/soc/cirrus/Kconfig"
 source "sound/soc/davinci/Kconfig"
 source "sound/soc/dwc/Kconfig"
 source "sound/soc/fsl/Kconfig"
+source "sound/soc/hda/Kconfig"
 source "sound/soc/jz4740/Kconfig"
 source "sound/soc/nuc900/Kconfig"
 source "sound/soc/omap/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 974ba708b482..8741d6a38bf6 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -21,6 +21,7 @@  obj-$(CONFIG_SND_SOC)	+= cirrus/
 obj-$(CONFIG_SND_SOC)	+= davinci/
 obj-$(CONFIG_SND_SOC)	+= dwc/
 obj-$(CONFIG_SND_SOC)	+= fsl/
+obj-$(CONFIG_SND_SOC)	+= hda/
 obj-$(CONFIG_SND_SOC)	+= jz4740/
 obj-$(CONFIG_SND_SOC)	+= intel/
 obj-$(CONFIG_SND_SOC)	+= mxs/
diff --git a/sound/soc/hda/Kconfig b/sound/soc/hda/Kconfig
new file mode 100644
index 000000000000..815943360bc5
--- /dev/null
+++ b/sound/soc/hda/Kconfig
@@ -0,0 +1,3 @@ 
+config SND_SOC_HDA_CORE
+	tristate
+	select SND_HDA_CORE
diff --git a/sound/soc/hda/Makefile b/sound/soc/hda/Makefile
new file mode 100644
index 000000000000..9585ab180a55
--- /dev/null
+++ b/sound/soc/hda/Makefile
@@ -0,0 +1,3 @@ 
+snd-soc-hda-core-objs := soc-hda-codec.o
+
+obj-$(CONFIG_SND_SOC_HDA_CORE) += snd-soc-hda-core.o
diff --git a/sound/soc/hda/soc-hda-codec.c b/sound/soc/hda/soc-hda-codec.c
new file mode 100644
index 000000000000..caceb2136bba
--- /dev/null
+++ b/sound/soc/hda/soc-hda-codec.c
@@ -0,0 +1,85 @@ 
+/*
+ * soc-hda-codec.c ASoC High Definition Audio Codec interface Definitions
+ *
+ * Copyright (c) 2014-2015 Intel Corporation
+ *
+ * Author(s): Jeeja KP <jeeja.kp@intel.com>
+ *
+ *
+ *  This driver is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This driver is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <sound/hdaudio.h>
+#include "soc-hda-codec.h"
+
+/**
+ * snd_soc_hda_get_device_id - gets the hdac device id entry
+ * @hdev: HD-audio core bus
+ * @drv: HD-audio core stream object to initialize
+ *
+ * Compares the hdac device vendor_id and revision_id to the hdac_soc_codec
+ * driver id_table and returns the matching device id entry.
+ */
+const struct soc_hda_device_id *
+snd_soc_hda_get_device_id(
+			struct hdac_device *hdev,
+			struct soc_hda_codec_driver *drv)
+{
+	if (drv->id_table) {
+		const struct soc_hda_device_id *id  = drv->id_table;
+
+		while (id->name[0]) {
+			if (hdev->vendor_id == id->id &&
+				(!id->rev_id || id->rev_id == hdev->revision_id))
+				return id;
+			id++;
+		}
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(snd_soc_hda_get_device_id);
+
+static int hda_codec_match(struct hdac_device *dev, struct hdac_driver *drv)
+{
+	struct soc_hda_codec_driver *driver = to_soc_hda_codec_driver(drv);
+
+	if (snd_soc_hda_get_device_id(dev, driver))
+		return 1;
+	else
+		return 0;
+}
+
+/**
+ * snd_soc_hda_codec_driver_register - register hda codec driver
+ * @drv: HD Audio soc codec driver
+ */
+int snd_soc_hda_codec_driver_register(struct soc_hda_codec_driver *drv)
+{
+	drv->core.driver.bus = &snd_hda_bus_type;
+	drv->core.type = HDA_DEV_ASOC;
+	drv->core.match = hda_codec_match;
+	return driver_register(&drv->core.driver);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hda_codec_driver_register);
+
+/**
+ * snd_soc_hda_codec_driver_unregister - unregister hda codec driver
+ * @drv: HD Audio soc codec driver
+ */
+void snd_soc_hda_codec_driver_unregister(struct soc_hda_codec_driver *drv)
+{
+	driver_unregister(&drv->core.driver);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hda_codec_driver_unregister);
+
+MODULE_DESCRIPTION("ASoC HDA codec core");
+MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/hda/soc-hda-codec.h b/sound/soc/hda/soc-hda-codec.h
new file mode 100644
index 000000000000..f8a199a58926
--- /dev/null
+++ b/sound/soc/hda/soc-hda-codec.h
@@ -0,0 +1,44 @@ 
+/*
+ * ASoC High Definition Audio Codec interface
+ *
+ * Copyright (c) 2014-2015 Intel Corporation
+ *
+ * Author(s): Jeeja KP <jeeja.kp@intel.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License as published by the Free
+ *  Software Foundation; either version 2 of the License, or (at your option)
+ *  any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT
+ *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ *  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ *  more details.
+ *
+ */
+
+#include <sound/hdaudio.h>
+
+/*HDA device table*/
+#define HDA_NAME_SIZE      20
+struct soc_hda_device_id {
+	__u32 id;
+	__u32 rev_id;
+	char name[HDA_NAME_SIZE];
+	unsigned long driver_data;
+};
+
+struct soc_hda_codec_driver {
+	struct hdac_driver core;
+	const struct soc_hda_device_id *id_table;
+};
+
+#define to_hdac_driver(drv) (container_of((drv), \
+				 struct hdac_driver, driver))
+#define to_soc_hda_codec_driver(hdrv) (container_of((hdrv), \
+				 struct soc_hda_codec_driver, core))
+
+int snd_soc_hda_codec_driver_register(struct soc_hda_codec_driver *drv);
+void snd_soc_hda_codec_driver_unregister(struct soc_hda_codec_driver *drv);
+const struct soc_hda_device_id *snd_soc_hda_get_device_id(
+	struct hdac_device *hdev, struct soc_hda_codec_driver *drv);