| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aae644dd-0469-3ffc-61f4-7e3396636a93@linux.intel.com>
Date: Mon, 19 Oct 2020 13:50:02 -0500
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To: Keith Tzneg <matsufan@...il.com>, alsa-devel@...a-project.org
Cc: Cezary Rojewski <cezary.rojewski@...el.com>,
Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
Jie Yang <yang.jie@...ux.intel.com>,
Mark Brown <broonie@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>,
Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com>,
Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
Naveen Manohar <naveen.m@...el.com>,
Yong Zhi <yong.zhi@...el.com>,
Libin Yang <libin.yang@...ux.intel.com>,
Krzysztof Kozlowski <krzk@...nel.org>,
Bard Liao <yung-chuan.liao@...ux.intel.com>,
Sathyanarayana Nujella <sathyanarayana.nujella@...el.com>,
Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
Fred Oh <fred.oh@...ux.intel.com>,
Mac Chiang <mac.chiang@...el.com>,
Rander Wang <rander.wang@...ux.intel.com>,
Keith Tzeng <keith.tzeng@...nta.corp-partner.google.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ASoC: Intel: boards: Add CML_RT1015 m/c driver
On 10/19/20 10:58 AM, Keith Tzneg wrote:
> From: Keith Tzeng <keith.tzeng@...nta.corp-partner.google.com>
>
> Machine driver to enable RT5682 on SSP0, DMIC, HDMI and RT1015 AMP on
> SSP1: Enabled 4 CH TDM playback with 24 bit data.
>
> Signed-off-by: Keith Tzeng <keith.tzeng@...nta.corp-partner.google.com>
> ---
> sound/soc/intel/boards/Kconfig | 17 +
> sound/soc/intel/boards/Makefile | 2 +
> sound/soc/intel/boards/cml_rt1015_rt5682.c | 570 ++++++++++++++++++++++
> sound/soc/intel/common/soc-acpi-intel-cnl-match.c | 7 +
> 4 files changed, 596 insertions(+)
> create mode 100644 sound/soc/intel/boards/cml_rt1015_rt5682.c
>
> diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
> index c10c378..9331d0a 100644
> --- a/sound/soc/intel/boards/Kconfig
> +++ b/sound/soc/intel/boards/Kconfig
> @@ -496,6 +496,23 @@ config SND_SOC_INTEL_SOF_CML_RT1011_RT5682_MACH
> Say Y if you have such a device.
> If unsure select "N".
>
> +config SND_SOC_INTEL_SOF_RT5682_MACH
> + tristate "SOF with rt5682 codec in I2S Mode"
> + depends on I2C && ACPI && GPIOLIB
> + depends on ((SND_HDA_CODEC_HDMI && SND_SOC_SOF_HDA_AUDIO_CODEC) &&\
> + (MFD_INTEL_LPSS || COMPILE_TEST)) ||\
> + (SND_SOC_SOF_BAYTRAIL && (X86_INTEL_LPSS || COMPILE_TEST))
> + select SND_SOC_MAX98373_I2C
> + select SND_SOC_RT1015
> + select SND_SOC_RT5682_I2C
> + select SND_SOC_DMIC
> + select SND_SOC_HDAC_HDMI
> + help
> + This adds support for ASoC machine driver for SOF platforms
> + with rt5682 codec.
> + Say Y if you have such a device.
> + If unsure select "N".
> +
This is duplicating an existing config
if SND_SOC_SOF_HDA_LINK || SND_SOC_SOF_BAYTRAIL
config SND_SOC_INTEL_SOF_RT5682_MACH
tristate "SOF with rt5682 codec in I2S Mode"
depends on I2C && ACPI && GPIOLIB
What you would need is to add
select SND_SOC_RT1015
to the existing config, not duplicate it.
> endif ## SND_SOC_SOF_COMETLAKE && SND_SOC_SOF_HDA_LINK
>
> if SND_SOC_SOF_JASPERLAKE
> diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
> index a58e4d2..73131cc 100644
> --- a/sound/soc/intel/boards/Makefile
> +++ b/sound/soc/intel/boards/Makefile
> @@ -20,6 +20,7 @@ snd-soc-sst-byt-cht-es8316-objs := bytcht_es8316.o
> snd-soc-sst-byt-cht-nocodec-objs := bytcht_nocodec.o
> snd-soc-sof_rt5682-objs := sof_rt5682.o hda_dsp_common.o sof_maxim_common.o
> snd-soc-cml_rt1011_rt5682-objs := cml_rt1011_rt5682.o hda_dsp_common.o
> +snd-soc-cml_rt1015_rt5682-objs := cml_rt1015_rt5682.o
> snd-soc-kbl_da7219_max98357a-objs := kbl_da7219_max98357a.o
> snd-soc-kbl_da7219_max98927-objs := kbl_da7219_max98927.o
> snd-soc-kbl_rt5663_max98927-objs := kbl_rt5663_max98927.o
> @@ -60,6 +61,7 @@ obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_DA7213_MACH) += snd-soc-sst-byt-cht-da7213.o
> obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_ES8316_MACH) += snd-soc-sst-byt-cht-es8316.o
> obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH) += snd-soc-sst-byt-cht-nocodec.o
> obj-$(CONFIG_SND_SOC_INTEL_SOF_CML_RT1011_RT5682_MACH) += snd-soc-cml_rt1011_rt5682.o
> +obj-$(CONFIG_SND_SOC_INTEL_SOF_CML_RT1015_RT5682_MACH) += cml_rt1015_rt5682.o
But here you are using a config that was never defined.
And as I said in my previous email, please consider reusing an existing
machine driver rather than adding a new one just to use rt1011 to rt1015.
> obj-$(CONFIG_SND_SOC_INTEL_KBL_DA7219_MAX98357A_MACH) += snd-soc-kbl_da7219_max98357a.o
> obj-$(CONFIG_SND_SOC_INTEL_KBL_DA7219_MAX98927_MACH) += snd-soc-kbl_da7219_max98927.o
> obj-$(CONFIG_SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH) += snd-soc-kbl_rt5663_max98927.o
> diff --git a/sound/soc/intel/boards/cml_rt1015_rt5682.c b/sound/soc/intel/boards/cml_rt1015_rt5682.c
> new file mode 100644
> index 0000000..bf18830
> --- /dev/null
> +++ b/sound/soc/intel/boards/cml_rt1015_rt5682.c
> @@ -0,0 +1,570 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright(c) 2019 Intel Corporation.
> +
> +/*
> + * Intel Cometlake I2S Machine driver for RT1015 + RT5682 codec
> + */
> +
> +#include <linux/input.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/dmi.h>
> +#include <linux/slab.h>
> +#include <asm/cpu_device_id.h>
> +#include <linux/acpi.h>
> +#include <sound/core.h>
> +#include <sound/jack.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +#include <sound/rt5682.h>
> +#include <sound/soc-acpi.h>
> +#include "../../codecs/rt1015.h"
> +#include "../../codecs/rt5682.h"
> +#include "../../codecs/hdac_hdmi.h"
> +
> +/* The platform clock outputs 24Mhz clock to codec as I2S MCLK */
> +#define CML_PLAT_CLK 24000000
> +#define CML_RT1015_CODEC_DAI "rt1015-aif"
> +#define CML_RT5682_CODEC_DAI "rt5682-aif1"
> +#define NAME_SIZE 32
> +
> +#define SOF_RT1015_SPEAKER_WL BIT(0)
> +#define SOF_RT1015_SPEAKER_WR BIT(1)
> +#define SOF_RT1015_SPEAKER_TL BIT(2)
> +#define SOF_RT1015_SPEAKER_TR BIT(3)
> +#define SPK_CH 4
> +
> +/* Default: Woofer speakers */
> +static unsigned long sof_rt1015_quirk = SOF_RT1015_SPEAKER_WL |
> + SOF_RT1015_SPEAKER_WR;
> +
> +static int sof_rt1015_quirk_cb(const struct dmi_system_id *id)
> +{
> + sof_rt1015_quirk = (unsigned long)id->driver_data;
> + return 1;
> +}
> +
> +static const struct dmi_system_id sof_rt1015_quirk_table[] = {
> + {
> + .callback = sof_rt1015_quirk_cb,
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Google"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Helios"),
> + },
> + .driver_data = (void *)(SOF_RT1015_SPEAKER_WL | SOF_RT1015_SPEAKER_WR |
> + SOF_RT1015_SPEAKER_TL | SOF_RT1015_SPEAKER_TR),
> + },
> + {
> + }
> +};
in cml_rt1011_rt5682.c, we already have:
static const struct dmi_system_id sof_rt1011_quirk_table[] = {
{
.callback = sof_rt1011_quirk_cb,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Google"),
DMI_MATCH(DMI_PRODUCT_NAME, "Helios"),
},
.driver_data = (void *)(SOF_RT1011_SPEAKER_WL | SOF_RT1011_SPEAKER_WR |
SOF_RT1011_SPEAKER_TL | SOF_RT1011_SPEAKER_TR),
},
{
}
};
Can someone explain how the same "Helios" product can have DMI identical
quirks pointing to different amplifiers? this really begs the question
on what PRODUCT_NAME means. I would expect DMI quirks to be unique, not
match in different drivers as suggested.
NAK on this version. Please don't copy-paste like this.
Powered by blists - more mailing lists