From patchwork Thu Dec 21 15:36:47 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Ughreja, Rakesh A" X-Patchwork-Id: 10127867 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7577E60318 for ; Thu, 21 Dec 2017 15:46:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6550229C6A for ; Thu, 21 Dec 2017 15:46:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 57C1F29CF2; Thu, 21 Dec 2017 15:46:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EFFC829C6A for ; Thu, 21 Dec 2017 15:46:22 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id D646F267877; Thu, 21 Dec 2017 16:37:00 +0100 (CET) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 74C38267879; Thu, 21 Dec 2017 16:36:59 +0100 (CET) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by alsa0.perex.cz (Postfix) with ESMTP id E696C266BBC for ; Thu, 21 Dec 2017 16:36:54 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Dec 2017 07:36:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,436,1508828400"; d="scan'208";a="4547721" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga007.jf.intel.com with ESMTP; 21 Dec 2017 07:36:51 -0800 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 21 Dec 2017 07:36:51 -0800 Received: from bgsmsx102.gar.corp.intel.com (10.223.4.172) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 21 Dec 2017 07:36:51 -0800 Received: from bgsmsx104.gar.corp.intel.com ([169.254.5.23]) by BGSMSX102.gar.corp.intel.com ([10.223.4.172]) with mapi id 14.03.0319.002; Thu, 21 Dec 2017 21:06:47 +0530 From: "Ughreja, Rakesh A" To: Mark Brown , Takashi Iwai Thread-Topic: [alsa-devel] [RFC v3 06/11] ASoC: hdac_hda: add ASoC based HDA codec driver Thread-Index: AQHTeX0QgpbM3RGcoU+S0R9OV9iC46NMDdMggAHfRhA= Date: Thu, 21 Dec 2017 15:36:47 +0000 Message-ID: <85DFEED57DC57344B2483EF7BF8CB60579ADF4DE@BGSMSX104.gar.corp.intel.com> References: <85DFEED57DC57344B2483EF7BF8CB60579ADB65F@BGSMSX104.gar.corp.intel.com> <85DFEED57DC57344B2483EF7BF8CB60579ADCE9A@BGSMSX104.gar.corp.intel.com> <85DFEED57DC57344B2483EF7BF8CB60579ADD339@BGSMSX104.gar.corp.intel.com> <85DFEED57DC57344B2483EF7BF8CB60579ADD712@BGSMSX104.gar.corp.intel.com> <85DFEED57DC57344B2483EF7BF8CB60579ADD8AC@BGSMSX104.gar.corp.intel.com> <20171220102625.GB17890@sirena.org.uk> <85DFEED57DC57344B2483EF7BF8CB60579ADE906@BGSMSX104.gar.corp.intel.com> In-Reply-To: <85DFEED57DC57344B2483EF7BF8CB60579ADE906@BGSMSX104.gar.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTYxMDY5Y2YtZmYxNS00OTljLTg0NzgtMjU3MGIzZDE5MmVhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJlS01lNTR0RGhRWHFBUFVVSisrWFRXbTFDRnBqaFBGSGJBR0lXQjZXMjlJQThoNm5QY3ExNEVyRVFPRkR3OXVvIn0= dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.223.10.10] MIME-Version: 1.0 Cc: "Koul, Vinod" , "liam.r.girdwood@linux.intel.com" , "alsa-devel@alsa-project.org" , "pierre-louis.bossart@linux.intel.com" , Patches Audio Subject: Re: [alsa-devel] [RFC v3 06/11] ASoC: hdac_hda: add ASoC based HDA codec driver X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP >-----Original Message----- >From: Ughreja, Rakesh A >Sent: Wednesday, December 20, 2017 4:23 PM >To: Mark Brown ; Takashi Iwai >Cc: alsa-devel@alsa-project.org; Koul, Vinod ; pierre- >louis.bossart@linux.intel.com; liam.r.girdwood@linux.intel.com; Patches Audio > >Subject: RE: [alsa-devel] [RFC v3 06/11] ASoC: hdac_hda: add ASoC based HDA >codec driver > >>> Well, honestly speaking, that complete morphing scares me. If it's a >>> simple additional call, it's easier to track the code flow. But your >>> patch essentially fakes the legacy codec and bus structs and twists >>> the control by that switch... which looks too fragile to me. >> >>Especially when it's layered on top of all the other x86 code which is >>also complicated and fragile - feels like there's lots of places things >>can go wrong and lots of potential unexpected interactions. > >Thanks, for the feedback Mark and Takashi. I will send the test patch >based on the suggestions from Takashi within couple of days. > Hi Takashi/Mark, I tried working on the concept and it turned out to be much simpler than what I thought. I added a type field for the bus which can be queried at runtime by the legacy codec driver and based on the type it can call the callbacks of the hdac_hda library. With this change the hdac_hda.c file is just a passive library and not a driver, which provides APIs which are called by the legacy codec driver to work with ASoC platform drivers. There are only two callbacks required, i.e. probe and remove. This is to register and unregister the asoc codec. Let me know if this is the right way to go. I can send you the full series once you are okay with the direction. --- test patch below -- This patch adds bus type field in hdac_bus structure to distinguish the bus instance. Bus allocated using snd_hdac_bus_init API sets the bus type to HDA_BUS_LEGACY and snd_hdac_ext_bus_init API sets it to HDA_BUS_EXT. codec drivers can identify the bus type by calling snd_hdac_get_bus_type API. Signed-off-by: Rakesh Ughreja --- include/sound/hdaudio.h | 15 +++++++++++++++ sound/hda/ext/hdac_ext_bus.c | 1 + sound/hda/hdac_bus.c | 1 + sound/pci/hda/hda_bind.c | 14 ++++++++++++++ sound/pci/hda/hda_codec.h | 12 ++++++++++++ 5 files changed, 43 insertions(+) diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index 30ec1b0..9db6a11 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -97,6 +97,12 @@ enum { HDA_DEV_ASOC, }; +enum { + HDA_BUS_INVALID, + HDA_BUS_LEGACY, + HDA_BUS_EXT, +}; + /* direction */ enum { HDA_INPUT, HDA_OUTPUT @@ -265,6 +271,7 @@ struct hdac_rb { */ struct hdac_bus { struct device *dev; + int type; const struct hdac_bus_ops *ops; const struct hdac_io_ops *io_ops; @@ -341,6 +348,14 @@ struct hdac_bus { }; +static inline int snd_hdac_get_bus_type(struct hdac_bus *bus) +{ + if (bus) + return bus->type; + else + return HDA_BUS_INVALID; +} + 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); diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c index e4bcb76..9d002da 100644 --- a/sound/hda/ext/hdac_ext_bus.c +++ b/sound/hda/ext/hdac_ext_bus.c @@ -102,6 +102,7 @@ int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device *dev, if (ret < 0) return ret; + bus->type = HDA_BUS_EXT; INIT_LIST_HEAD(&bus->hlink_list); bus->idx = idx++; diff --git a/sound/hda/hdac_bus.c b/sound/hda/hdac_bus.c index 714a517..bb7567d 100644 --- a/sound/hda/hdac_bus.c +++ b/sound/hda/hdac_bus.c @@ -35,6 +35,7 @@ int snd_hdac_bus_init(struct hdac_bus *bus, struct device *dev, else bus->ops = &default_ops; bus->io_ops = io_ops; + bus->type = HDA_BUS_LEGACY; INIT_LIST_HEAD(&bus->stream_list); INIT_LIST_HEAD(&bus->codec_list); INIT_WORK(&bus->unsol_work, process_unsol_events); diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c index d361bb7..1998376 100644 --- a/sound/pci/hda/hda_bind.c +++ b/sound/pci/hda/hda_bind.c @@ -81,6 +81,15 @@ static int hda_codec_driver_probe(struct device *dev) hda_codec_patch_t patch; int err; +#if IS_ENABLED(CONFIG_SND_SOC_HDAC_HDA) + dev_err(dev, "BUS Type = %d\n", snd_hdac_get_bus_type(&codec->bus->core)); + if (HDA_BUS_EXT == snd_hdac_get_bus_type(&codec->bus->core)) { + if (!codec->ext_ops) + return -EINVAL; + return codec->ext_ops->probe(&codec->core); + } +#endif + if (WARN_ON(!codec->preset)) return -EINVAL; @@ -134,6 +143,11 @@ static int hda_codec_driver_remove(struct device *dev) { struct hda_codec *codec = dev_to_hda_codec(dev); +#if IS_ENABLED(CONFIG_SND_SOC_HDAC_HDA) + if (HDA_BUS_EXT == snd_hdac_get_bus_type(&codec->bus->core)) { + return codec->ext_ops->remove(&codec->core); + } +#endif if (codec->patch_ops.free) codec->patch_ops.free(codec); snd_hda_codec_cleanup_for_unbind(codec); diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index 8bbedf7..2b7b7db 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -108,6 +108,13 @@ void hda_codec_driver_unregister(struct hda_codec_driver *drv); module_driver(drv, hda_codec_driver_register, \ hda_codec_driver_unregister) +#if IS_ENABLED(CONFIG_SND_SOC_HDAC_HDA) +struct snd_hda_ext_ops { + int (*probe)(struct hdac_device *hdev); + int (*remove)(struct hdac_device *hdev); +}; +#endif + /* ops set by the preset patch */ struct hda_codec_ops { int (*build_controls)(struct hda_codec *codec); @@ -289,6 +296,11 @@ struct hda_codec { /* additional init verbs */ struct snd_array verbs; + +#if IS_ENABLED(CONFIG_SND_SOC_HDAC_HDA) + struct snd_hda_ext_ops *ext_ops; +#endif + }; #define dev_to_hda_codec(_dev) container_of(_dev, struct hda_codec, core.dev)