From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 69B2CC433EF for ; Wed, 24 Nov 2021 07:12:18 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 97AA61741; Wed, 24 Nov 2021 08:11:26 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 97AA61741 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1637737936; bh=NvRS+PxUadnJIEWZSazLz+A/6XNjK15Mm5ZywOo5WPw=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=NmT/w/f5pdupM9kJwL1khQZ+rWseIwIXphOQt2lsfcYps5W9yBAFEEYpxh65vyWWP VLS3/6dcMFxKyZsvLlkdLp+HZvXQ9osghssNFVXyMe7O4Yh3RHwF84Y+N2sPuiBmgN FeXP6P/i6aelUvF29WBls38+ur8LGrVEJs9DmG0g= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 809EBF8057B; Wed, 24 Nov 2021 08:06:10 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 11097F8049E; Tue, 23 Nov 2021 17:52:43 +0100 (CET) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 12A9FF80154 for ; Tue, 23 Nov 2021 17:52:35 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 12A9FF80154 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="foDFfOER"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="jWoC58AF" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 32B331FD58; Tue, 23 Nov 2021 16:52:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1637686355; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=YAE9GMutJ4y4QSEVMvaF/1s6v5n7VJFmuzuv2HkXzkU=; b=foDFfOERDf2ZANAXUgsXk7ltAXGsxPFbbc/bpg55D0X6G5X5YitFK0ZzE4p4ktlRbRbA7y 0P3tT0dhz1qwfO/TyXYl2nUBkLCBBLv76qyxARWgcEAVQt0ABApMAF7jCITWUOpwRrnZiL jJn0Utfb+li45acqDlOClaGz0wAgD0U= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1637686355; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=YAE9GMutJ4y4QSEVMvaF/1s6v5n7VJFmuzuv2HkXzkU=; b=jWoC58AFYXJTjETKZY+8Ox7IfOg0MK5p0GVz1K9h/FB2O5L9CXTghK8gmtzNQkKbMqkjml VjOETtdD2C751pDg== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id B3DCFA3B8B; Tue, 23 Nov 2021 16:52:34 +0000 (UTC) Date: Tue, 23 Nov 2021 17:52:34 +0100 Message-ID: From: Takashi Iwai To: Lucas Tanure Subject: Re: [PATCH 10/11] hda: cs35l41: Add support for CS35L41 in HDA systems In-Reply-To: <20211123163149.1530535-11-tanureal@opensource.cirrus.com> References: <20211123163149.1530535-1-tanureal@opensource.cirrus.com> <20211123163149.1530535-11-tanureal@opensource.cirrus.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII X-Mailman-Approved-At: Wed, 24 Nov 2021 08:05:53 +0100 Cc: Alexandre Belloni , Elia Devito , "Rafael J . Wysocki" , alsa-devel@alsa-project.org, Werner Sembach , Liam Girdwood , Shuming Fan , Lars-Peter Clausen , Vitaly Rodionov , Jeremy Szu , Pierre-Louis Bossart , Hui Wang , linux-acpi@vger.kernel.org, Sami Loone , Len Brown , platform-driver-x86@vger.kernel.org, Chris Chiu , Arnd Bergmann , Mark Gross , Hans de Goede , Mark Brown , Cameron Berkenpas , Jack Yu , Kailang Yang , patches@opensource.cirrus.com, Takashi Iwai , linux-kernel@vger.kernel.org, David Rhodes X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 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" On Tue, 23 Nov 2021 17:31:48 +0100, Lucas Tanure wrote: > > --- a/sound/pci/hda/Makefile > +++ b/sound/pci/hda/Makefile > @@ -13,25 +13,27 @@ snd-hda-codec-$(CONFIG_SND_HDA_INPUT_BEEP) += hda_beep.o > CFLAGS_hda_controller.o := -I$(src) > CFLAGS_hda_intel.o := -I$(src) > > -snd-hda-codec-generic-objs := hda_generic.o > -snd-hda-codec-realtek-objs := patch_realtek.o > -snd-hda-codec-cmedia-objs := patch_cmedia.o > -snd-hda-codec-analog-objs := patch_analog.o > -snd-hda-codec-idt-objs := patch_sigmatel.o > -snd-hda-codec-si3054-objs := patch_si3054.o > -snd-hda-codec-cirrus-objs := patch_cirrus.o > -snd-hda-codec-cs8409-objs := patch_cs8409.o patch_cs8409-tables.o > -snd-hda-codec-ca0110-objs := patch_ca0110.o > -snd-hda-codec-ca0132-objs := patch_ca0132.o > -snd-hda-codec-conexant-objs := patch_conexant.o > -snd-hda-codec-via-objs := patch_via.o > -snd-hda-codec-hdmi-objs := patch_hdmi.o hda_eld.o > +snd-hda-codec-generic-objs := hda_generic.o You don't need to change other lines because of the newly added driver below... > +snd-hda-codec-cs35l41-i2c-objs := cs35l41_hda_i2c.o cs35l41_hda.o ../../soc/codecs/cs35l41-lib.o Linking the object in a different level of directory is too ugly and would be problematic if multiple drivers want the cs35l41-lib stuff. IMO, it's better to make symbols in cs35l41-lib exported and select the corresponding Kconfig from HD-audio driver. And, snd-hda-codec-cs35l41-i2c is not really a codec driver. It's rather some bridge for i2c over HD-audio. So, better to avoid snd-hda-codec-* but have some different name. Otherwise people may misunderstand. > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c (snip) > @@ -6497,6 +6502,98 @@ static void alc287_fixup_legion_15imhg05_speakers(struct hda_codec *codec, > } > } > > +static int comp_match_dev_name(struct device *dev, void *data) > +{ > + if (strcmp(dev_name(dev), data) == 0) > + return 1; > + > + return 0; > +} > + > +static int find_comp_by_dev_name(struct alc_spec *spec, const char *name) > +{ > + int i; > + > + for (i = 0; i < HDA_MAX_COMPONENTS; i++) { > + if (strcmp(spec->comps[i].name, name) == 0) > + return i; > + } > + > + return -ENODEV; > +} > + > +static int comp_bind(struct device *dev) > +{ > + struct hda_codec *codec = dev_to_hda_codec(dev); > + struct alc_spec *spec = codec->spec; > + > + return component_bind_all(dev, spec->comps); > +} > + > +static void comp_unbind(struct device *dev) > +{ > + struct hda_codec *codec = dev_to_hda_codec(dev); > + struct alc_spec *spec = codec->spec; > + > + component_unbind_all(dev, spec->comps); > +} > + > +static const struct component_master_ops comp_master_ops = { > + .bind = comp_bind, > + .unbind = comp_unbind, > +}; > + > +void alc287_legion_16achg6_playback_hook(struct hda_pcm_stream *hinfo, struct hda_codec *codec, > + struct snd_pcm_substream *sub, int action) > +{ > + struct alc_spec *spec = codec->spec; > + unsigned int rx_slot; > + int i = 0; > + > + switch (action) { > + case HDA_GEN_PCM_ACT_PREPARE: > + rx_slot = 0; > + i = find_comp_by_dev_name(spec, "i2c-CLSA0100:00-cs35l41-hda.0"); > + if (i >= 0) > + spec->comps[i].set_channel_map(spec->comps[i].dev, 0, NULL, 1, &rx_slot); > + > + rx_slot = 1; > + i = find_comp_by_dev_name(spec, "i2c-CLSA0100:00-cs35l41-hda.1"); > + if (i >= 0) > + spec->comps[i].set_channel_map(spec->comps[i].dev, 0, NULL, 1, &rx_slot); > + break; > + } > + > + for (i = 0; i < HDA_MAX_COMPONENTS; i++) { > + if (spec->comps[i].dev) > + spec->comps[i].playback_hook(spec->comps[i].dev, action); > + } > + > + > +} > + > +static void alc287_fixup_legion_16achg6_speakers(struct hda_codec *codec, > + const struct hda_fixup *fix, int action) > +{ > + struct device *dev = hda_codec_dev(codec); > + struct alc_spec *spec = codec->spec; > + int ret; > + > + switch (action) { > + case HDA_FIXUP_ACT_PRE_PROBE: > + component_match_add(dev, &spec->match, comp_match_dev_name, > + "i2c-CLSA0100:00-cs35l41-hda.0"); > + component_match_add(dev, &spec->match, comp_match_dev_name, > + "i2c-CLSA0100:00-cs35l41-hda.1"); > + ret = component_master_add_with_match(dev, &comp_master_ops, spec->match); > + if (ret) > + codec_err(codec, "Fail to register component aggregator %d\n", ret); > + else > + spec->gen.pcm_playback_hook = alc287_legion_16achg6_playback_hook; > + break; > + } > +} > + Those are needed only if the new cs35l41 stuff is enabled, so they can be wrapped with #if IS_REACHABLE(xxx). thanks, Takashi From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 420C1C433FE for ; Tue, 23 Nov 2021 16:52:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232419AbhKWQzp (ORCPT ); Tue, 23 Nov 2021 11:55:45 -0500 Received: from smtp-out2.suse.de ([195.135.220.29]:52686 "EHLO smtp-out2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229825AbhKWQzp (ORCPT ); Tue, 23 Nov 2021 11:55:45 -0500 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 32B331FD58; Tue, 23 Nov 2021 16:52:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1637686355; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=YAE9GMutJ4y4QSEVMvaF/1s6v5n7VJFmuzuv2HkXzkU=; b=foDFfOERDf2ZANAXUgsXk7ltAXGsxPFbbc/bpg55D0X6G5X5YitFK0ZzE4p4ktlRbRbA7y 0P3tT0dhz1qwfO/TyXYl2nUBkLCBBLv76qyxARWgcEAVQt0ABApMAF7jCITWUOpwRrnZiL jJn0Utfb+li45acqDlOClaGz0wAgD0U= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1637686355; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=YAE9GMutJ4y4QSEVMvaF/1s6v5n7VJFmuzuv2HkXzkU=; b=jWoC58AFYXJTjETKZY+8Ox7IfOg0MK5p0GVz1K9h/FB2O5L9CXTghK8gmtzNQkKbMqkjml VjOETtdD2C751pDg== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id B3DCFA3B8B; Tue, 23 Nov 2021 16:52:34 +0000 (UTC) Date: Tue, 23 Nov 2021 17:52:34 +0100 Message-ID: From: Takashi Iwai To: Lucas Tanure Cc: "Rafael J . Wysocki" , Len Brown , Hans de Goede , Mark Gross , "Liam Girdwood" , Jaroslav Kysela , Mark Brown , Takashi Iwai , Kailang Yang , Shuming Fan , "Pierre-Louis Bossart" , David Rhodes , Vitaly Rodionov , Jeremy Szu , Hui Wang , Werner Sembach , Chris Chiu , Cameron Berkenpas , Sami Loone , Elia Devito , Srinivas Kandagatla , Jack Yu , "Arnd Bergmann" , Lars-Peter Clausen , "Alexandre Belloni" , , , , , Subject: Re: [PATCH 10/11] hda: cs35l41: Add support for CS35L41 in HDA systems In-Reply-To: <20211123163149.1530535-11-tanureal@opensource.cirrus.com> References: <20211123163149.1530535-1-tanureal@opensource.cirrus.com> <20211123163149.1530535-11-tanureal@opensource.cirrus.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Tue, 23 Nov 2021 17:31:48 +0100, Lucas Tanure wrote: > > --- a/sound/pci/hda/Makefile > +++ b/sound/pci/hda/Makefile > @@ -13,25 +13,27 @@ snd-hda-codec-$(CONFIG_SND_HDA_INPUT_BEEP) += hda_beep.o > CFLAGS_hda_controller.o := -I$(src) > CFLAGS_hda_intel.o := -I$(src) > > -snd-hda-codec-generic-objs := hda_generic.o > -snd-hda-codec-realtek-objs := patch_realtek.o > -snd-hda-codec-cmedia-objs := patch_cmedia.o > -snd-hda-codec-analog-objs := patch_analog.o > -snd-hda-codec-idt-objs := patch_sigmatel.o > -snd-hda-codec-si3054-objs := patch_si3054.o > -snd-hda-codec-cirrus-objs := patch_cirrus.o > -snd-hda-codec-cs8409-objs := patch_cs8409.o patch_cs8409-tables.o > -snd-hda-codec-ca0110-objs := patch_ca0110.o > -snd-hda-codec-ca0132-objs := patch_ca0132.o > -snd-hda-codec-conexant-objs := patch_conexant.o > -snd-hda-codec-via-objs := patch_via.o > -snd-hda-codec-hdmi-objs := patch_hdmi.o hda_eld.o > +snd-hda-codec-generic-objs := hda_generic.o You don't need to change other lines because of the newly added driver below... > +snd-hda-codec-cs35l41-i2c-objs := cs35l41_hda_i2c.o cs35l41_hda.o ../../soc/codecs/cs35l41-lib.o Linking the object in a different level of directory is too ugly and would be problematic if multiple drivers want the cs35l41-lib stuff. IMO, it's better to make symbols in cs35l41-lib exported and select the corresponding Kconfig from HD-audio driver. And, snd-hda-codec-cs35l41-i2c is not really a codec driver. It's rather some bridge for i2c over HD-audio. So, better to avoid snd-hda-codec-* but have some different name. Otherwise people may misunderstand. > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c (snip) > @@ -6497,6 +6502,98 @@ static void alc287_fixup_legion_15imhg05_speakers(struct hda_codec *codec, > } > } > > +static int comp_match_dev_name(struct device *dev, void *data) > +{ > + if (strcmp(dev_name(dev), data) == 0) > + return 1; > + > + return 0; > +} > + > +static int find_comp_by_dev_name(struct alc_spec *spec, const char *name) > +{ > + int i; > + > + for (i = 0; i < HDA_MAX_COMPONENTS; i++) { > + if (strcmp(spec->comps[i].name, name) == 0) > + return i; > + } > + > + return -ENODEV; > +} > + > +static int comp_bind(struct device *dev) > +{ > + struct hda_codec *codec = dev_to_hda_codec(dev); > + struct alc_spec *spec = codec->spec; > + > + return component_bind_all(dev, spec->comps); > +} > + > +static void comp_unbind(struct device *dev) > +{ > + struct hda_codec *codec = dev_to_hda_codec(dev); > + struct alc_spec *spec = codec->spec; > + > + component_unbind_all(dev, spec->comps); > +} > + > +static const struct component_master_ops comp_master_ops = { > + .bind = comp_bind, > + .unbind = comp_unbind, > +}; > + > +void alc287_legion_16achg6_playback_hook(struct hda_pcm_stream *hinfo, struct hda_codec *codec, > + struct snd_pcm_substream *sub, int action) > +{ > + struct alc_spec *spec = codec->spec; > + unsigned int rx_slot; > + int i = 0; > + > + switch (action) { > + case HDA_GEN_PCM_ACT_PREPARE: > + rx_slot = 0; > + i = find_comp_by_dev_name(spec, "i2c-CLSA0100:00-cs35l41-hda.0"); > + if (i >= 0) > + spec->comps[i].set_channel_map(spec->comps[i].dev, 0, NULL, 1, &rx_slot); > + > + rx_slot = 1; > + i = find_comp_by_dev_name(spec, "i2c-CLSA0100:00-cs35l41-hda.1"); > + if (i >= 0) > + spec->comps[i].set_channel_map(spec->comps[i].dev, 0, NULL, 1, &rx_slot); > + break; > + } > + > + for (i = 0; i < HDA_MAX_COMPONENTS; i++) { > + if (spec->comps[i].dev) > + spec->comps[i].playback_hook(spec->comps[i].dev, action); > + } > + > + > +} > + > +static void alc287_fixup_legion_16achg6_speakers(struct hda_codec *codec, > + const struct hda_fixup *fix, int action) > +{ > + struct device *dev = hda_codec_dev(codec); > + struct alc_spec *spec = codec->spec; > + int ret; > + > + switch (action) { > + case HDA_FIXUP_ACT_PRE_PROBE: > + component_match_add(dev, &spec->match, comp_match_dev_name, > + "i2c-CLSA0100:00-cs35l41-hda.0"); > + component_match_add(dev, &spec->match, comp_match_dev_name, > + "i2c-CLSA0100:00-cs35l41-hda.1"); > + ret = component_master_add_with_match(dev, &comp_master_ops, spec->match); > + if (ret) > + codec_err(codec, "Fail to register component aggregator %d\n", ret); > + else > + spec->gen.pcm_playback_hook = alc287_legion_16achg6_playback_hook; > + break; > + } > +} > + Those are needed only if the new cs35l41 stuff is enabled, so they can be wrapped with #if IS_REACHABLE(xxx). thanks, Takashi