[PATCH 00/17] ASoC: Intel: haswell and broadwell boards update
A number of patches improving overall quality and readability of haswell.c and broadwell.c source files found in sound/soc/intel/boards. Both files are first renamed and only then actual changes are being incrementally added. The respective names are: hsw_rt5640 and bdw_rt286 to match the pattern found in more recent boards.
Most patches bring no functional change - the more impactful patches at are placed the end:
Refactor of suspend/resume flow for the bdw_rt286 board by dropping dev->remove() in favour of card->remove() and adjust jack handling to reduce code size slightly by implementing card_set_jack().
The last patch is removing of FE DAI ops. Given the existence of platform FE DAI capabilities (either static declaration or through topology file), this code is redundant.
Cezary Rojewski (17): ASoC: Intel: Rename haswell source file to hsw_rt5640 ASoC: Intel: hsw_rt5640: Reword prefixes of all driver members ASoC: Intel: hsw_rt5640: Reword driver name ASoC: Intel: hsw_rt5640: Update code indentation ASoC: Intel: hsw_rt5640: Update file comments ASoC: Intel: hsw_rt5640: Improve probe() function quality ASoC: Intel: hsw_rt5640: Improve hw_params() debug-ability ASoC: Intel: Rename broadwell source file to bdw_rt286 ASoC: Intel: bdw_rt286: Reword prefixes of all driver members ASoC: Intel: bdw_rt286: Reword driver name ASoC: Intel: bdw_rt286: Update code indentation ASoC: Intel: bdw_rt286: Update file comments ASoC: Intel: bdw_rt286: Improve probe() function quality ASoC: Intel: bdw_rt286: Improve hw_params() debug-ability ASoC: Intel: bdw_rt286: Improve codec_init() quality ASoC: Intel: bdw_rt286: Refactor suspend/resume ASoC: Intel: bdw_rt286: Remove FE DAI ops
sound/soc/intel/boards/Kconfig | 4 +- sound/soc/intel/boards/Makefile | 4 +- sound/soc/intel/boards/bdw_rt286.c | 257 +++++++++++++ sound/soc/intel/boards/broadwell.c | 338 ------------------ sound/soc/intel/boards/haswell.c | 202 ----------- sound/soc/intel/boards/hsw_rt5640.c | 176 +++++++++ .../common/soc-acpi-intel-hsw-bdw-match.c | 6 +- 7 files changed, 440 insertions(+), 547 deletions(-) create mode 100644 sound/soc/intel/boards/bdw_rt286.c delete mode 100644 sound/soc/intel/boards/broadwell.c delete mode 100644 sound/soc/intel/boards/haswell.c create mode 100644 sound/soc/intel/boards/hsw_rt5640.c
Rename source file to drop any ambiguity.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/Makefile | 2 +- sound/soc/intel/boards/{haswell.c => hsw_rt5640.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename sound/soc/intel/boards/{haswell.c => hsw_rt5640.c} (100%)
diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile index 40c0c3d1c500..e479546a3d4b 100644 --- a/sound/soc/intel/boards/Makefile +++ b/sound/soc/intel/boards/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -snd-soc-sst-haswell-objs := haswell.o +snd-soc-sst-haswell-objs := hsw_rt5640.o snd-soc-sst-bdw-rt5650-mach-objs := bdw-rt5650.o snd-soc-sst-bdw-rt5677-mach-objs := bdw-rt5677.o snd-soc-sst-broadwell-objs := broadwell.o diff --git a/sound/soc/intel/boards/haswell.c b/sound/soc/intel/boards/hsw_rt5640.c similarity index 100% rename from sound/soc/intel/boards/haswell.c rename to sound/soc/intel/boards/hsw_rt5640.c
Replace ambiguous 'broadwell_rt286_' prefixes in favour of 'card_', 'link_' and other similar strings to clearly state which object given member implements behavior for.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/hsw_rt5640.c | 46 ++++++++++++++--------------- 1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c index aa61e101f793..b51ce8d0ca22 100644 --- a/sound/soc/intel/boards/hsw_rt5640.c +++ b/sound/soc/intel/boards/hsw_rt5640.c @@ -16,12 +16,12 @@ #include "../../codecs/rt5640.h"
/* Haswell ULT platforms have a Headphone and Mic jack */ -static const struct snd_soc_dapm_widget haswell_widgets[] = { +static const struct snd_soc_dapm_widget card_widgets[] = { SND_SOC_DAPM_HP("Headphones", NULL), SND_SOC_DAPM_MIC("Mic", NULL), };
-static const struct snd_soc_dapm_route haswell_rt5640_map[] = { +static const struct snd_soc_dapm_route card_routes[] = {
{"Headphones", NULL, "HPOR"}, {"Headphones", NULL, "HPOL"}, @@ -32,7 +32,7 @@ static const struct snd_soc_dapm_route haswell_rt5640_map[] = { {"AIF1 Playback", NULL, "SSP0 CODEC OUT"}, };
-static int haswell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd, +static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params) { struct snd_interval *rate = hw_param_interval(params, @@ -49,7 +49,7 @@ static int haswell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd, return 0; }
-static int haswell_rt5640_hw_params(struct snd_pcm_substream *substream, +static int codec_link_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); @@ -70,8 +70,8 @@ static int haswell_rt5640_hw_params(struct snd_pcm_substream *substream, return ret; }
-static const struct snd_soc_ops haswell_rt5640_ops = { - .hw_params = haswell_rt5640_hw_params, +static const struct snd_soc_ops codec_link_ops = { + .hw_params = codec_link_hw_params, };
SND_SOC_DAILINK_DEF(dummy, @@ -98,7 +98,7 @@ SND_SOC_DAILINK_DEF(platform, SND_SOC_DAILINK_DEF(ssp0_port, DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
-static struct snd_soc_dai_link haswell_rt5640_dais[] = { +static struct snd_soc_dai_link card_dai_links[] = { /* Front End DAI links */ { .name = "System", @@ -147,8 +147,8 @@ static struct snd_soc_dai_link haswell_rt5640_dais[] = { .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC, .ignore_pmdown_time = 1, - .be_hw_params_fixup = haswell_ssp0_fixup, - .ops = &haswell_rt5640_ops, + .be_hw_params_fixup = codec_link_hw_params_fixup, + .ops = &codec_link_ops, .dpcm_playback = 1, .dpcm_capture = 1, SND_SOC_DAILINK_REG(ssp0_port, codec, platform), @@ -156,44 +156,44 @@ static struct snd_soc_dai_link haswell_rt5640_dais[] = { };
/* audio machine driver for Haswell Lynxpoint DSP + RT5640 */ -static struct snd_soc_card haswell_rt5640 = { +static struct snd_soc_card hsw_rt5640_card = { .name = "haswell-rt5640", .owner = THIS_MODULE, - .dai_link = haswell_rt5640_dais, - .num_links = ARRAY_SIZE(haswell_rt5640_dais), - .dapm_widgets = haswell_widgets, - .num_dapm_widgets = ARRAY_SIZE(haswell_widgets), - .dapm_routes = haswell_rt5640_map, - .num_dapm_routes = ARRAY_SIZE(haswell_rt5640_map), + .dai_link = card_dai_links, + .num_links = ARRAY_SIZE(card_dai_links), + .dapm_widgets = card_widgets, + .num_dapm_widgets = ARRAY_SIZE(card_widgets), + .dapm_routes = card_routes, + .num_dapm_routes = ARRAY_SIZE(card_routes), .fully_routed = true, };
-static int haswell_audio_probe(struct platform_device *pdev) +static int hsw_rt5640_probe(struct platform_device *pdev) { struct snd_soc_acpi_mach *mach; int ret;
- haswell_rt5640.dev = &pdev->dev; + hsw_rt5640_card.dev = &pdev->dev;
/* override platform name, if required */ mach = pdev->dev.platform_data; - ret = snd_soc_fixup_dai_links_platform_name(&haswell_rt5640, + ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card, mach->mach_params.platform); if (ret) return ret;
- return devm_snd_soc_register_card(&pdev->dev, &haswell_rt5640); + return devm_snd_soc_register_card(&pdev->dev, &hsw_rt5640_card); }
-static struct platform_driver haswell_audio = { - .probe = haswell_audio_probe, +static struct platform_driver hsw_rt5640_driver = { + .probe = hsw_rt5640_probe, .driver = { .name = "haswell-audio", .pm = &snd_soc_pm_ops, }, };
-module_platform_driver(haswell_audio) +module_platform_driver(hsw_rt5640_driver)
/* Module information */ MODULE_AUTHOR("Liam Girdwood, Xingchao Wang");
On 2022-06-10 2:36 PM, Cezary Rojewski wrote:
Replace ambiguous 'broadwell_rt286_' prefixes in favour of 'card_', 'link_' and other similar strings to clearly state which object given member implements behavior for.
That's a prime copy+paste example right here. Should be 'haswell_rt5640_' for hsw_rt5640 board. I'll add this reword to the changes request during the review, should there be any.
Regards, Czarek
Align with other Intel boards naming convention and let the name explicitly state which components are being connected.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/hsw_rt5640.c | 4 ++-- sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c index b51ce8d0ca22..a096453bf1df 100644 --- a/sound/soc/intel/boards/hsw_rt5640.c +++ b/sound/soc/intel/boards/hsw_rt5640.c @@ -188,7 +188,7 @@ static int hsw_rt5640_probe(struct platform_device *pdev) static struct platform_driver hsw_rt5640_driver = { .probe = hsw_rt5640_probe, .driver = { - .name = "haswell-audio", + .name = "hsw_rt5640", .pm = &snd_soc_pm_ops, }, }; @@ -199,4 +199,4 @@ module_platform_driver(hsw_rt5640_driver) MODULE_AUTHOR("Liam Girdwood, Xingchao Wang"); MODULE_DESCRIPTION("Intel SST Audio for Haswell Lynxpoint"); MODULE_LICENSE("GPL v2"); -MODULE_ALIAS("platform:haswell-audio"); +MODULE_ALIAS("platform:hsw_rt5640"); diff --git a/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c b/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c index 0441df97b260..4e00f8f6c521 100644 --- a/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c +++ b/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c @@ -12,7 +12,7 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_haswell_machines[] = { { .id = "INT33CA", - .drv_name = "haswell-audio", + .drv_name = "hsw_rt5640", .fw_filename = "intel/IntcSST1.bin", .sof_tplg_filename = "sof-hsw.tplg", }, @@ -41,7 +41,7 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_broadwell_machines[] = { }, { .id = "INT33CA", - .drv_name = "haswell-audio", + .drv_name = "hsw_rt5640", .fw_filename = "intel/IntcSST2.bin", .sof_tplg_filename = "sof-bdw-rt5640.tplg", },
Make use of 100 character limit and modify indentation so code is easier to read. While at it, sort includes in alphabetical order.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/hsw_rt5640.c | 67 ++++++++++------------------- 1 file changed, 22 insertions(+), 45 deletions(-)
diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c index a096453bf1df..69be5086b98d 100644 --- a/sound/soc/intel/boards/hsw_rt5640.c +++ b/sound/soc/intel/boards/hsw_rt5640.c @@ -9,10 +9,9 @@ #include <linux/platform_device.h> #include <sound/core.h> #include <sound/pcm.h> +#include <sound/pcm_params.h> #include <sound/soc.h> #include <sound/soc-acpi.h> -#include <sound/pcm_params.h> - #include "../../codecs/rt5640.h"
/* Haswell ULT platforms have a Headphone and Mic jack */ @@ -22,7 +21,6 @@ static const struct snd_soc_dapm_widget card_widgets[] = { };
static const struct snd_soc_dapm_route card_routes[] = { - {"Headphones", NULL, "HPOR"}, {"Headphones", NULL, "HPOL"}, {"IN2P", NULL, "Mic"}, @@ -33,32 +31,28 @@ static const struct snd_soc_dapm_route card_routes[] = { };
static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, - struct snd_pcm_hw_params *params) + struct snd_pcm_hw_params *params) { - struct snd_interval *rate = hw_param_interval(params, - SNDRV_PCM_HW_PARAM_RATE); - struct snd_interval *channels = hw_param_interval(params, - SNDRV_PCM_HW_PARAM_CHANNELS); + struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); + struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
/* The ADSP will covert the FE rate to 48k, stereo */ rate->min = rate->max = 48000; channels->min = channels->max = 2; - /* set SSP0 to 16 bit */ params_set_format(params, SNDRV_PCM_FORMAT_S16_LE); + return 0; }
static int codec_link_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params) + struct snd_pcm_hw_params *params) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0); int ret;
- ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_MCLK, 12288000, - SND_SOC_CLOCK_IN); - + ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_MCLK, 12288000, SND_SOC_CLOCK_IN); if (ret < 0) { dev_err(rtd->dev, "can't set codec sysclk configuration\n"); return ret; @@ -74,29 +68,15 @@ static const struct snd_soc_ops codec_link_ops = { .hw_params = codec_link_hw_params, };
-SND_SOC_DAILINK_DEF(dummy, - DAILINK_COMP_ARRAY(COMP_DUMMY())); - -SND_SOC_DAILINK_DEF(system, - DAILINK_COMP_ARRAY(COMP_CPU("System Pin"))); +SND_SOC_DAILINK_DEF(system, DAILINK_COMP_ARRAY(COMP_CPU("System Pin"))); +SND_SOC_DAILINK_DEF(offload0, DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin"))); +SND_SOC_DAILINK_DEF(offload1, DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin"))); +SND_SOC_DAILINK_DEF(loopback, DAILINK_COMP_ARRAY(COMP_CPU("Loopback Pin")));
-SND_SOC_DAILINK_DEF(offload0, - DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin"))); - -SND_SOC_DAILINK_DEF(offload1, - DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin"))); - -SND_SOC_DAILINK_DEF(loopback, - DAILINK_COMP_ARRAY(COMP_CPU("Loopback Pin"))); - -SND_SOC_DAILINK_DEF(codec, - DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT33CA:00", "rt5640-aif1"))); - -SND_SOC_DAILINK_DEF(platform, - DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audio"))); - -SND_SOC_DAILINK_DEF(ssp0_port, - DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port"))); +SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY())); +SND_SOC_DAILINK_DEF(codec, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT33CA:00", "rt5640-aif1"))); +SND_SOC_DAILINK_DEF(platform, DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audio"))); +SND_SOC_DAILINK_DEF(ssp0_port, DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
static struct snd_soc_dai_link card_dai_links[] = { /* Front End DAI links */ @@ -105,7 +85,7 @@ static struct snd_soc_dai_link card_dai_links[] = { .stream_name = "System Playback/Capture", .nonatomic = 1, .dynamic = 1, - .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, + .trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST }, .dpcm_playback = 1, .dpcm_capture = 1, SND_SOC_DAILINK_REG(system, dummy, platform), @@ -115,7 +95,7 @@ static struct snd_soc_dai_link card_dai_links[] = { .stream_name = "Offload0 Playback", .nonatomic = 1, .dynamic = 1, - .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, + .trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST }, .dpcm_playback = 1, SND_SOC_DAILINK_REG(offload0, dummy, platform), }, @@ -124,7 +104,7 @@ static struct snd_soc_dai_link card_dai_links[] = { .stream_name = "Offload1 Playback", .nonatomic = 1, .dynamic = 1, - .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, + .trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST }, .dpcm_playback = 1, SND_SOC_DAILINK_REG(offload1, dummy, platform), }, @@ -133,19 +113,17 @@ static struct snd_soc_dai_link card_dai_links[] = { .stream_name = "Loopback", .nonatomic = 1, .dynamic = 1, - .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, + .trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST }, .dpcm_capture = 1, SND_SOC_DAILINK_REG(loopback, dummy, platform), }, - /* Back End DAI links */ { /* SSP0 - Codec */ .name = "Codec", .id = 0, .no_pcm = 1, - .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | - SND_SOC_DAIFMT_CBC_CFC, + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC, .ignore_pmdown_time = 1, .be_hw_params_fixup = codec_link_hw_params_fixup, .ops = &codec_link_ops, @@ -174,11 +152,10 @@ static int hsw_rt5640_probe(struct platform_device *pdev) int ret;
hsw_rt5640_card.dev = &pdev->dev; - /* override platform name, if required */ mach = pdev->dev.platform_data; - ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card, - mach->mach_params.platform); + + ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card, mach->mach_params.platform); if (ret) return ret;
Drop redundant and update valuable comments within the file to increase readability. This patch also revisits module information and kconfig help strings.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/Kconfig | 2 +- sound/soc/intel/boards/hsw_rt5640.c | 16 ++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index 4b4c1e1e4808..817b4c04bf6a 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -41,7 +41,7 @@ config SND_SOC_INTEL_SOF_CIRRUS_COMMON if SND_SOC_INTEL_CATPT
config SND_SOC_INTEL_HASWELL_MACH - tristate "Haswell Lynxpoint" + tristate "Haswell with RT5640 I2S codec" depends on I2C depends on I2C_DESIGNWARE_PLATFORM || COMPILE_TEST depends on X86_INTEL_LPSS || COMPILE_TEST diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c index 69be5086b98d..0cd788a73694 100644 --- a/sound/soc/intel/boards/hsw_rt5640.c +++ b/sound/soc/intel/boards/hsw_rt5640.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Intel Haswell Lynxpoint SST Audio + * Sound card driver for Intel Haswell Lynx Point with Realtek 5640 * * Copyright (C) 2013, Intel Corporation. All rights reserved. */ @@ -14,7 +14,6 @@ #include <sound/soc-acpi.h> #include "../../codecs/rt5640.h"
-/* Haswell ULT platforms have a Headphone and Mic jack */ static const struct snd_soc_dapm_widget card_widgets[] = { SND_SOC_DAPM_HP("Headphones", NULL), SND_SOC_DAPM_MIC("Mic", NULL), @@ -36,10 +35,10 @@ static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
- /* The ADSP will covert the FE rate to 48k, stereo */ + /* The ADSP will convert the FE rate to 48k, stereo. */ rate->min = rate->max = 48000; channels->min = channels->max = 2; - /* set SSP0 to 16 bit */ + /* Set SSP0 to 16 bit. */ params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
return 0; @@ -58,7 +57,7 @@ static int codec_link_hw_params(struct snd_pcm_substream *substream, return ret; }
- /* set correct codec filter for DAI format and clock config */ + /* Set correct codec filter for DAI format and clock config. */ snd_soc_component_update_bits(codec_dai->component, 0x83, 0xffff, 0x8000);
return ret; @@ -133,7 +132,6 @@ static struct snd_soc_dai_link card_dai_links[] = { }, };
-/* audio machine driver for Haswell Lynxpoint DSP + RT5640 */ static struct snd_soc_card hsw_rt5640_card = { .name = "haswell-rt5640", .owner = THIS_MODULE, @@ -152,7 +150,6 @@ static int hsw_rt5640_probe(struct platform_device *pdev) int ret;
hsw_rt5640_card.dev = &pdev->dev; - /* override platform name, if required */ mach = pdev->dev.platform_data;
ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card, mach->mach_params.platform); @@ -172,8 +169,7 @@ static struct platform_driver hsw_rt5640_driver = {
module_platform_driver(hsw_rt5640_driver)
-/* Module information */ MODULE_AUTHOR("Liam Girdwood, Xingchao Wang"); -MODULE_DESCRIPTION("Intel SST Audio for Haswell Lynxpoint"); -MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Sound card driver for Intel Haswell Lynx Point with Realtek 5640"); +MODULE_LICENSE("GPL"); MODULE_ALIAS("platform:hsw_rt5640");
Declare local 'dev' and make use of it plus dev_get_platdata() to improve code readability.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/hsw_rt5640.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c index 0cd788a73694..94f96de0b62a 100644 --- a/sound/soc/intel/boards/hsw_rt5640.c +++ b/sound/soc/intel/boards/hsw_rt5640.c @@ -147,16 +147,17 @@ static struct snd_soc_card hsw_rt5640_card = { static int hsw_rt5640_probe(struct platform_device *pdev) { struct snd_soc_acpi_mach *mach; + struct device *dev = &pdev->dev; int ret;
- hsw_rt5640_card.dev = &pdev->dev; - mach = pdev->dev.platform_data; + hsw_rt5640_card.dev = dev; + mach = dev_get_platdata(dev);
ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card, mach->mach_params.platform); if (ret) return ret;
- return devm_snd_soc_register_card(&pdev->dev, &hsw_rt5640_card); + return devm_snd_soc_register_card(dev, &hsw_rt5640_card); }
static struct platform_driver hsw_rt5640_driver = {
Print status if setting sysclk fails.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/hsw_rt5640.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c index 94f96de0b62a..5e2224f7e986 100644 --- a/sound/soc/intel/boards/hsw_rt5640.c +++ b/sound/soc/intel/boards/hsw_rt5640.c @@ -53,7 +53,7 @@ static int codec_link_hw_params(struct snd_pcm_substream *substream,
ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_MCLK, 12288000, SND_SOC_CLOCK_IN); if (ret < 0) { - dev_err(rtd->dev, "can't set codec sysclk configuration\n"); + dev_err(rtd->dev, "set codec sysclk failed: %d\n", ret); return ret; }
Rename source file to drop any ambiguity.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/Makefile | 2 +- sound/soc/intel/boards/{broadwell.c => bdw_rt286.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename sound/soc/intel/boards/{broadwell.c => bdw_rt286.c} (100%)
diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile index e479546a3d4b..eea1e26acfda 100644 --- a/sound/soc/intel/boards/Makefile +++ b/sound/soc/intel/boards/Makefile @@ -2,7 +2,7 @@ snd-soc-sst-haswell-objs := hsw_rt5640.o snd-soc-sst-bdw-rt5650-mach-objs := bdw-rt5650.o snd-soc-sst-bdw-rt5677-mach-objs := bdw-rt5677.o -snd-soc-sst-broadwell-objs := broadwell.o +snd-soc-sst-broadwell-objs := bdw_rt286.o snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o snd-soc-sst-bxt-rt298-objs := bxt_rt298.o snd-soc-sst-sof-pcm512x-objs := sof_pcm512x.o diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/bdw_rt286.c similarity index 100% rename from sound/soc/intel/boards/broadwell.c rename to sound/soc/intel/boards/bdw_rt286.c
Replace ambiguous 'broadwell_rt286_' prefixes in favour of 'card_', 'link_' and other similar strings to clearly state which object given member implements behavior for.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/bdw_rt286.c | 96 +++++++++++++++--------------- 1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index b29d77dfb281..092bc9a45548 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -16,9 +16,9 @@
#include "../../codecs/rt286.h"
-static struct snd_soc_jack broadwell_headset; +static struct snd_soc_jack card_headset; /* Headset jack detection DAPM pins */ -static struct snd_soc_jack_pin broadwell_headset_pins[] = { +static struct snd_soc_jack_pin card_headset_pins[] = { { .pin = "Mic Jack", .mask = SND_JACK_MICROPHONE, @@ -29,12 +29,12 @@ static struct snd_soc_jack_pin broadwell_headset_pins[] = { }, };
-static const struct snd_kcontrol_new broadwell_controls[] = { +static const struct snd_kcontrol_new card_controls[] = { SOC_DAPM_PIN_SWITCH("Speaker"), SOC_DAPM_PIN_SWITCH("Headphone Jack"), };
-static const struct snd_soc_dapm_widget broadwell_widgets[] = { +static const struct snd_soc_dapm_widget card_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", NULL), SND_SOC_DAPM_SPK("Speaker", NULL), SND_SOC_DAPM_MIC("Mic Jack", NULL), @@ -43,7 +43,7 @@ static const struct snd_soc_dapm_widget broadwell_widgets[] = { SND_SOC_DAPM_LINE("Line Jack", NULL), };
-static const struct snd_soc_dapm_route broadwell_rt286_map[] = { +static const struct snd_soc_dapm_route card_routes[] = {
/* speaker */ {"Speaker", NULL, "SPOR"}, @@ -65,22 +65,22 @@ static const struct snd_soc_dapm_route broadwell_rt286_map[] = { {"AIF1 Playback", NULL, "SSP0 CODEC OUT"}, };
-static int broadwell_rt286_codec_init(struct snd_soc_pcm_runtime *rtd) +static int codec_link_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; int ret = 0; ret = snd_soc_card_jack_new_pins(rtd->card, "Headset", - SND_JACK_HEADSET | SND_JACK_BTN_0, &broadwell_headset, - broadwell_headset_pins, ARRAY_SIZE(broadwell_headset_pins)); + SND_JACK_HEADSET | SND_JACK_BTN_0, &card_headset, + card_headset_pins, ARRAY_SIZE(card_headset_pins)); if (ret) return ret;
- rt286_mic_detect(component, &broadwell_headset); + rt286_mic_detect(component, &card_headset); return 0; }
-static int broadwell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd, +static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params) { struct snd_interval *rate = hw_param_interval(params, @@ -97,7 +97,7 @@ static int broadwell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd, return 0; }
-static int broadwell_rt286_hw_params(struct snd_pcm_substream *substream, +static int codec_link_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); @@ -115,8 +115,8 @@ static int broadwell_rt286_hw_params(struct snd_pcm_substream *substream, return ret; }
-static const struct snd_soc_ops broadwell_rt286_ops = { - .hw_params = broadwell_rt286_hw_params, +static const struct snd_soc_ops codec_link_ops = { + .hw_params = codec_link_hw_params, };
static const unsigned int channels[] = { @@ -129,7 +129,7 @@ static const struct snd_pcm_hw_constraint_list constraints_channels = { .mask = 0, };
-static int broadwell_fe_startup(struct snd_pcm_substream *substream) +static int bdw_rt286_fe_startup(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime;
@@ -140,8 +140,8 @@ static int broadwell_fe_startup(struct snd_pcm_substream *substream) &constraints_channels); }
-static const struct snd_soc_ops broadwell_fe_ops = { - .startup = broadwell_fe_startup, +static const struct snd_soc_ops bdw_rt286_fe_ops = { + .startup = bdw_rt286_fe_startup, };
SND_SOC_DAILINK_DEF(system, @@ -169,7 +169,7 @@ SND_SOC_DAILINK_DEF(ssp0_port, DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
/* broadwell digital audio interface glue - connects codec <--> CPU */ -static struct snd_soc_dai_link broadwell_rt286_dais[] = { +static struct snd_soc_dai_link card_dai_links[] = { /* Front End DAI links */ { .name = "System PCM", @@ -177,7 +177,7 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = { .nonatomic = 1, .dynamic = 1, .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, - .ops = &broadwell_fe_ops, + .ops = &bdw_rt286_fe_ops, .dpcm_playback = 1, .dpcm_capture = 1, SND_SOC_DAILINK_REG(system, dummy, platform), @@ -215,12 +215,12 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = { .name = "Codec", .id = 0, .no_pcm = 1, - .init = broadwell_rt286_codec_init, + .init = codec_link_init, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC, .ignore_pmdown_time = 1, - .be_hw_params_fixup = broadwell_ssp0_fixup, - .ops = &broadwell_rt286_ops, + .be_hw_params_fixup = codec_link_hw_params_fixup, + .ops = &codec_link_ops, .dpcm_playback = 1, .dpcm_capture = 1, SND_SOC_DAILINK_REG(ssp0_port, codec, platform), @@ -241,21 +241,21 @@ static void broadwell_disable_jack(struct snd_soc_card *card) } }
-static int broadwell_suspend(struct snd_soc_card *card) +static int card_suspend_pre(struct snd_soc_card *card) { broadwell_disable_jack(card);
return 0; }
-static int broadwell_resume(struct snd_soc_card *card){ +static int card_resume_post(struct snd_soc_card *card){ struct snd_soc_component *component;
for_each_card_components(card, component) { if (!strcmp(component->name, "i2c-INT343A:00")) {
dev_dbg(component->dev, "enabling jack detect for resume.\n"); - rt286_mic_detect(component, &broadwell_headset); + rt286_mic_detect(component, &card_headset); break; } } @@ -270,48 +270,48 @@ static int broadwell_resume(struct snd_soc_card *card){ #define DRIVER_NAME NULL /* card name will be used for driver name */
/* broadwell audio machine driver for WPT + RT286S */ -static struct snd_soc_card broadwell_rt286 = { +static struct snd_soc_card bdw_rt286_card = { .owner = THIS_MODULE, - .dai_link = broadwell_rt286_dais, - .num_links = ARRAY_SIZE(broadwell_rt286_dais), - .controls = broadwell_controls, - .num_controls = ARRAY_SIZE(broadwell_controls), - .dapm_widgets = broadwell_widgets, - .num_dapm_widgets = ARRAY_SIZE(broadwell_widgets), - .dapm_routes = broadwell_rt286_map, - .num_dapm_routes = ARRAY_SIZE(broadwell_rt286_map), + .dai_link = card_dai_links, + .num_links = ARRAY_SIZE(card_dai_links), + .controls = card_controls, + .num_controls = ARRAY_SIZE(card_controls), + .dapm_widgets = card_widgets, + .num_dapm_widgets = ARRAY_SIZE(card_widgets), + .dapm_routes = card_routes, + .num_dapm_routes = ARRAY_SIZE(card_routes), .fully_routed = true, - .suspend_pre = broadwell_suspend, - .resume_post = broadwell_resume, + .suspend_pre = card_suspend_pre, + .resume_post = card_resume_post, };
-static int broadwell_audio_probe(struct platform_device *pdev) +static int bdw_rt286_probe(struct platform_device *pdev) { struct snd_soc_acpi_mach *mach; int ret;
- broadwell_rt286.dev = &pdev->dev; + bdw_rt286_card.dev = &pdev->dev;
/* override platform name, if required */ mach = pdev->dev.platform_data; - ret = snd_soc_fixup_dai_links_platform_name(&broadwell_rt286, + ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card, mach->mach_params.platform); if (ret) return ret;
/* set card and driver name */ if (snd_soc_acpi_sof_parent(&pdev->dev)) { - broadwell_rt286.name = SOF_CARD_NAME; - broadwell_rt286.driver_name = SOF_DRIVER_NAME; + bdw_rt286_card.name = SOF_CARD_NAME; + bdw_rt286_card.driver_name = SOF_DRIVER_NAME; } else { - broadwell_rt286.name = CARD_NAME; - broadwell_rt286.driver_name = DRIVER_NAME; + bdw_rt286_card.name = CARD_NAME; + bdw_rt286_card.driver_name = DRIVER_NAME; }
- return devm_snd_soc_register_card(&pdev->dev, &broadwell_rt286); + return devm_snd_soc_register_card(&pdev->dev, &bdw_rt286_card); }
-static int broadwell_audio_remove(struct platform_device *pdev) +static int bdw_rt286_remove(struct platform_device *pdev) { struct snd_soc_card *card = platform_get_drvdata(pdev);
@@ -320,16 +320,16 @@ static int broadwell_audio_remove(struct platform_device *pdev) return 0; }
-static struct platform_driver broadwell_audio = { - .probe = broadwell_audio_probe, - .remove = broadwell_audio_remove, +static struct platform_driver bdw_rt286_driver = { + .probe = bdw_rt286_probe, + .remove = bdw_rt286_remove, .driver = { .name = "broadwell-audio", .pm = &snd_soc_pm_ops }, };
-module_platform_driver(broadwell_audio) +module_platform_driver(bdw_rt286_driver)
/* Module information */ MODULE_AUTHOR("Liam Girdwood, Xingchao Wang");
On 6/10/22 07:36, Cezary Rojewski wrote:
Replace ambiguous 'broadwell_rt286_' prefixes in favour of 'card_', 'link_' and other similar strings to clearly state which object given member implements behavior for.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com
This doesn't apply or assumes something else was applied already?
sound/soc/intel/boards/bdw_rt286.c | 96 +++++++++++++++--------------- 1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index b29d77dfb281..092bc9a45548 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -16,9 +16,9 @@
#include "../../codecs/rt286.h"
-static struct snd_soc_jack broadwell_headset; +static struct snd_soc_jack card_headset; /* Headset jack detection DAPM pins */ -static struct snd_soc_jack_pin broadwell_headset_pins[] = { +static struct snd_soc_jack_pin card_headset_pins[] = { { .pin = "Mic Jack", .mask = SND_JACK_MICROPHONE, @@ -29,12 +29,12 @@ static struct snd_soc_jack_pin broadwell_headset_pins[] = { }, };
-static const struct snd_kcontrol_new broadwell_controls[] = { +static const struct snd_kcontrol_new card_controls[] = { SOC_DAPM_PIN_SWITCH("Speaker"), SOC_DAPM_PIN_SWITCH("Headphone Jack"), };
-static const struct snd_soc_dapm_widget broadwell_widgets[] = { +static const struct snd_soc_dapm_widget card_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", NULL), SND_SOC_DAPM_SPK("Speaker", NULL), SND_SOC_DAPM_MIC("Mic Jack", NULL), @@ -43,7 +43,7 @@ static const struct snd_soc_dapm_widget broadwell_widgets[] = { SND_SOC_DAPM_LINE("Line Jack", NULL), };
-static const struct snd_soc_dapm_route broadwell_rt286_map[] = { +static const struct snd_soc_dapm_route card_routes[] = {
/* speaker */ {"Speaker", NULL, "SPOR"}, @@ -65,22 +65,22 @@ static const struct snd_soc_dapm_route broadwell_rt286_map[] = { {"AIF1 Playback", NULL, "SSP0 CODEC OUT"}, };
-static int broadwell_rt286_codec_init(struct snd_soc_pcm_runtime *rtd) +static int codec_link_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; int ret = 0; ret = snd_soc_card_jack_new_pins(rtd->card, "Headset",
SND_JACK_HEADSET | SND_JACK_BTN_0, &broadwell_headset,broadwell_headset_pins, ARRAY_SIZE(broadwell_headset_pins));
SND_JACK_HEADSET | SND_JACK_BTN_0, &card_headset,card_headset_pins, ARRAY_SIZE(card_headset_pins));
- rt286_mic_detect(component, &broadwell_headset);
- rt286_mic_detect(component, &card_headset); return 0;
}
-static int broadwell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd, +static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params) { struct snd_interval *rate = hw_param_interval(params, @@ -97,7 +97,7 @@ static int broadwell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd, return 0; }
-static int broadwell_rt286_hw_params(struct snd_pcm_substream *substream, +static int codec_link_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); @@ -115,8 +115,8 @@ static int broadwell_rt286_hw_params(struct snd_pcm_substream *substream, return ret; }
-static const struct snd_soc_ops broadwell_rt286_ops = {
- .hw_params = broadwell_rt286_hw_params,
+static const struct snd_soc_ops codec_link_ops = {
- .hw_params = codec_link_hw_params,
};
static const unsigned int channels[] = { @@ -129,7 +129,7 @@ static const struct snd_pcm_hw_constraint_list constraints_channels = { .mask = 0, };
-static int broadwell_fe_startup(struct snd_pcm_substream *substream) +static int bdw_rt286_fe_startup(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime;
@@ -140,8 +140,8 @@ static int broadwell_fe_startup(struct snd_pcm_substream *substream) &constraints_channels); }
-static const struct snd_soc_ops broadwell_fe_ops = {
- .startup = broadwell_fe_startup,
+static const struct snd_soc_ops bdw_rt286_fe_ops = {
- .startup = bdw_rt286_fe_startup,
};
SND_SOC_DAILINK_DEF(system, @@ -169,7 +169,7 @@ SND_SOC_DAILINK_DEF(ssp0_port, DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
/* broadwell digital audio interface glue - connects codec <--> CPU */ -static struct snd_soc_dai_link broadwell_rt286_dais[] = { +static struct snd_soc_dai_link card_dai_links[] = { /* Front End DAI links */ { .name = "System PCM", @@ -177,7 +177,7 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = { .nonatomic = 1, .dynamic = 1, .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
.ops = &broadwell_fe_ops,
.ops = &bdw_rt286_fe_ops,@@ -215,12 +215,12 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = { .name = "Codec", .id = 0, .no_pcm = 1,
.init = broadwell_rt286_codec_init,
.init = codec_link_init,
.be_hw_params_fixup = broadwell_ssp0_fixup,.ops = &broadwell_rt286_ops,
.be_hw_params_fixup = codec_link_hw_params_fixup,.ops = &codec_link_ops,@@ -241,21 +241,21 @@ static void broadwell_disable_jack(struct snd_soc_card *card) } }
-static int broadwell_suspend(struct snd_soc_card *card) +static int card_suspend_pre(struct snd_soc_card *card) { broadwell_disable_jack(card);
return 0; }
-static int broadwell_resume(struct snd_soc_card *card){ +static int card_resume_post(struct snd_soc_card *card){ struct snd_soc_component *component;
for_each_card_components(card, component) { if (!strcmp(component->name, "i2c-INT343A:00")) {
dev_dbg(component->dev, "enabling jack detect for resume.\n");
rt286_mic_detect(component, &broadwell_headset);
rt286_mic_detect(component, &card_headset); break;@@ -270,48 +270,48 @@ static int broadwell_resume(struct snd_soc_card *card){ #define DRIVER_NAME NULL /* card name will be used for driver name */
/* broadwell audio machine driver for WPT + RT286S */ -static struct snd_soc_card broadwell_rt286 = { +static struct snd_soc_card bdw_rt286_card = { .owner = THIS_MODULE,
- .dai_link = broadwell_rt286_dais,
- .num_links = ARRAY_SIZE(broadwell_rt286_dais),
- .controls = broadwell_controls,
- .num_controls = ARRAY_SIZE(broadwell_controls),
- .dapm_widgets = broadwell_widgets,
- .num_dapm_widgets = ARRAY_SIZE(broadwell_widgets),
- .dapm_routes = broadwell_rt286_map,
- .num_dapm_routes = ARRAY_SIZE(broadwell_rt286_map),
- .dai_link = card_dai_links,
- .num_links = ARRAY_SIZE(card_dai_links),
- .controls = card_controls,
- .num_controls = ARRAY_SIZE(card_controls),
- .dapm_widgets = card_widgets,
- .num_dapm_widgets = ARRAY_SIZE(card_widgets),
- .dapm_routes = card_routes,
- .num_dapm_routes = ARRAY_SIZE(card_routes), .fully_routed = true,
- .suspend_pre = broadwell_suspend,
- .resume_post = broadwell_resume,
- .suspend_pre = card_suspend_pre,
- .resume_post = card_resume_post,
};
-static int broadwell_audio_probe(struct platform_device *pdev) +static int bdw_rt286_probe(struct platform_device *pdev) { struct snd_soc_acpi_mach *mach; int ret;
- broadwell_rt286.dev = &pdev->dev;
bdw_rt286_card.dev = &pdev->dev;
/* override platform name, if required */ mach = pdev->dev.platform_data;
- ret = snd_soc_fixup_dai_links_platform_name(&broadwell_rt286,
ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card, mach->mach_params.platform); if (ret) return ret;
/* set card and driver name */ if (snd_soc_acpi_sof_parent(&pdev->dev)) {
broadwell_rt286.name = SOF_CARD_NAME;broadwell_rt286.driver_name = SOF_DRIVER_NAME;
bdw_rt286_card.name = SOF_CARD_NAME;bdw_rt286_card.driver_name = SOF_DRIVER_NAME;
broadwell_rt286.name = CARD_NAME;broadwell_rt286.driver_name = DRIVER_NAME;
bdw_rt286_card.name = CARD_NAME;bdw_rt286_card.driver_name = DRIVER_NAME;
- return devm_snd_soc_register_card(&pdev->dev, &broadwell_rt286);
- return devm_snd_soc_register_card(&pdev->dev, &bdw_rt286_card);
}
-static int broadwell_audio_remove(struct platform_device *pdev) +static int bdw_rt286_remove(struct platform_device *pdev) { struct snd_soc_card *card = platform_get_drvdata(pdev);
@@ -320,16 +320,16 @@ static int broadwell_audio_remove(struct platform_device *pdev) return 0; }
-static struct platform_driver broadwell_audio = {
- .probe = broadwell_audio_probe,
- .remove = broadwell_audio_remove,
+static struct platform_driver bdw_rt286_driver = {
- .probe = bdw_rt286_probe,
- .remove = bdw_rt286_remove, .driver = { .name = "broadwell-audio", .pm = &snd_soc_pm_ops },
};
-module_platform_driver(broadwell_audio) +module_platform_driver(bdw_rt286_driver)
/* Module information */ MODULE_AUTHOR("Liam Girdwood, Xingchao Wang");
On 2022-06-10 6:34 PM, Pierre-Louis Bossart wrote:
On 6/10/22 07:36, Cezary Rojewski wrote:
Replace ambiguous 'broadwell_rt286_' prefixes in favour of 'card_', 'link_' and other similar strings to clearly state which object given member implements behavior for.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com
This doesn't apply or assumes something else was applied already?
I believe it conflicted with "ASoC: codecs: rt286: Reorganize jack detect handling" [1] as that patch updates the users of rt286_mic_detect(). That patch was just merged to broonie/sound/for-next. Forgotten about those few lines being modified there. Will update in v2.
[1]: https://lore.kernel.org/alsa-devel/20220609133541.3984886-3-amadeuszx.slawin...
Regards, Czarek
Align with other Intel boards naming convention and let the name explicitly state which components are being connected.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/bdw_rt286.c | 4 ++-- sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index 092bc9a45548..0e67741aea79 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -324,7 +324,7 @@ static struct platform_driver bdw_rt286_driver = { .probe = bdw_rt286_probe, .remove = bdw_rt286_remove, .driver = { - .name = "broadwell-audio", + .name = "bdw_rt286", .pm = &snd_soc_pm_ops }, }; @@ -335,4 +335,4 @@ module_platform_driver(bdw_rt286_driver) MODULE_AUTHOR("Liam Girdwood, Xingchao Wang"); MODULE_DESCRIPTION("Intel SST Audio for WPT/Broadwell"); MODULE_LICENSE("GPL v2"); -MODULE_ALIAS("platform:broadwell-audio"); +MODULE_ALIAS("platform:bdw_rt286"); diff --git a/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c b/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c index 4e00f8f6c521..cbcb649604e5 100644 --- a/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c +++ b/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c @@ -23,7 +23,7 @@ EXPORT_SYMBOL_GPL(snd_soc_acpi_intel_haswell_machines); struct snd_soc_acpi_mach snd_soc_acpi_intel_broadwell_machines[] = { { .id = "INT343A", - .drv_name = "broadwell-audio", + .drv_name = "bdw_rt286", .fw_filename = "intel/IntcSST2.bin", .sof_tplg_filename = "sof-bdw-rt286.tplg", },
Make use of 100 character limit and modify indentation so code is easier to read. While at it, sort includes in alphabetical order.
While at it, rename local variable 'chan' to 'channels' to match hsw_rt5640 board's equivalent.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/bdw_rt286.c | 87 +++++++++++------------------- 1 file changed, 32 insertions(+), 55 deletions(-)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index 0e67741aea79..fbef19a545b0 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -8,12 +8,11 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <sound/core.h> -#include <sound/pcm.h> -#include <sound/soc.h> #include <sound/jack.h> +#include <sound/pcm.h> #include <sound/pcm_params.h> +#include <sound/soc.h> #include <sound/soc-acpi.h> - #include "../../codecs/rt286.h"
static struct snd_soc_jack card_headset; @@ -44,7 +43,6 @@ static const struct snd_soc_dapm_widget card_widgets[] = { };
static const struct snd_soc_dapm_route card_routes[] = { - /* speaker */ {"Speaker", NULL, "SPOR"}, {"Speaker", NULL, "SPOL"}, @@ -69,9 +67,10 @@ static int codec_link_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; int ret = 0; - ret = snd_soc_card_jack_new_pins(rtd->card, "Headset", - SND_JACK_HEADSET | SND_JACK_BTN_0, &card_headset, - card_headset_pins, ARRAY_SIZE(card_headset_pins)); + + ret = snd_soc_card_jack_new_pins(rtd->card, "Headset", SND_JACK_HEADSET | SND_JACK_BTN_0, + &card_headset, card_headset_pins, + ARRAY_SIZE(card_headset_pins)); if (ret) return ret;
@@ -79,34 +78,29 @@ static int codec_link_init(struct snd_soc_pcm_runtime *rtd) return 0; }
- static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, - struct snd_pcm_hw_params *params) + struct snd_pcm_hw_params *params) { - struct snd_interval *rate = hw_param_interval(params, - SNDRV_PCM_HW_PARAM_RATE); - struct snd_interval *chan = hw_param_interval(params, - SNDRV_PCM_HW_PARAM_CHANNELS); + struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); + struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
/* The ADSP will covert the FE rate to 48k, stereo */ rate->min = rate->max = 48000; - chan->min = chan->max = 2; - + channels->min = channels->max = 2; /* set SSP0 to 16 bit */ params_set_format(params, SNDRV_PCM_FORMAT_S16_LE); + return 0; }
static int codec_link_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params) + struct snd_pcm_hw_params *params) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0); int ret;
- ret = snd_soc_dai_set_sysclk(codec_dai, RT286_SCLK_S_PLL, 24000000, - SND_SOC_CLOCK_IN); - + ret = snd_soc_dai_set_sysclk(codec_dai, RT286_SCLK_S_PLL, 24000000, SND_SOC_CLOCK_IN); if (ret < 0) { dev_err(rtd->dev, "can't set codec sysclk configuration\n"); return ret; @@ -135,8 +129,7 @@ static int bdw_rt286_fe_startup(struct snd_pcm_substream *substream)
/* Board supports stereo configuration only */ runtime->hw.channels_max = 2; - return snd_pcm_hw_constraint_list(runtime, 0, - SNDRV_PCM_HW_PARAM_CHANNELS, + return snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, &constraints_channels); }
@@ -144,29 +137,15 @@ static const struct snd_soc_ops bdw_rt286_fe_ops = { .startup = bdw_rt286_fe_startup, };
-SND_SOC_DAILINK_DEF(system, - DAILINK_COMP_ARRAY(COMP_CPU("System Pin"))); - -SND_SOC_DAILINK_DEF(offload0, - DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin"))); +SND_SOC_DAILINK_DEF(system, DAILINK_COMP_ARRAY(COMP_CPU("System Pin"))); +SND_SOC_DAILINK_DEF(offload0, DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin"))); +SND_SOC_DAILINK_DEF(offload1, DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin"))); +SND_SOC_DAILINK_DEF(loopback, DAILINK_COMP_ARRAY(COMP_CPU("Loopback Pin")));
-SND_SOC_DAILINK_DEF(offload1, - DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin"))); - -SND_SOC_DAILINK_DEF(loopback, - DAILINK_COMP_ARRAY(COMP_CPU("Loopback Pin"))); - -SND_SOC_DAILINK_DEF(dummy, - DAILINK_COMP_ARRAY(COMP_DUMMY())); - -SND_SOC_DAILINK_DEF(platform, - DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audio"))); - -SND_SOC_DAILINK_DEF(codec, - DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1"))); - -SND_SOC_DAILINK_DEF(ssp0_port, - DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port"))); +SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY())); +SND_SOC_DAILINK_DEF(platform, DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audio"))); +SND_SOC_DAILINK_DEF(codec, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1"))); +SND_SOC_DAILINK_DEF(ssp0_port, DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
/* broadwell digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link card_dai_links[] = { @@ -176,7 +155,7 @@ static struct snd_soc_dai_link card_dai_links[] = { .stream_name = "System Playback/Capture", .nonatomic = 1, .dynamic = 1, - .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, + .trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST }, .ops = &bdw_rt286_fe_ops, .dpcm_playback = 1, .dpcm_capture = 1, @@ -187,7 +166,7 @@ static struct snd_soc_dai_link card_dai_links[] = { .stream_name = "Offload0 Playback", .nonatomic = 1, .dynamic = 1, - .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, + .trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST }, .dpcm_playback = 1, SND_SOC_DAILINK_REG(offload0, dummy, platform), }, @@ -196,7 +175,7 @@ static struct snd_soc_dai_link card_dai_links[] = { .stream_name = "Offload1 Playback", .nonatomic = 1, .dynamic = 1, - .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, + .trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST }, .dpcm_playback = 1, SND_SOC_DAILINK_REG(offload1, dummy, platform), }, @@ -205,7 +184,7 @@ static struct snd_soc_dai_link card_dai_links[] = { .stream_name = "Loopback", .nonatomic = 1, .dynamic = 1, - .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, + .trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST }, .dpcm_capture = 1, SND_SOC_DAILINK_REG(loopback, dummy, platform), }, @@ -216,8 +195,7 @@ static struct snd_soc_dai_link card_dai_links[] = { .id = 0, .no_pcm = 1, .init = codec_link_init, - .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | - SND_SOC_DAIFMT_CBC_CFC, + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC, .ignore_pmdown_time = 1, .be_hw_params_fixup = codec_link_hw_params_fixup, .ops = &codec_link_ops, @@ -233,7 +211,6 @@ static void broadwell_disable_jack(struct snd_soc_card *card)
for_each_card_components(card, component) { if (!strcmp(component->name, "i2c-INT343A:00")) { - dev_dbg(component->dev, "disabling jack detect before going to suspend.\n"); rt286_mic_detect(component, NULL); break; @@ -248,17 +225,18 @@ static int card_suspend_pre(struct snd_soc_card *card) return 0; }
-static int card_resume_post(struct snd_soc_card *card){ +static int card_resume_post(struct snd_soc_card *card) +{ struct snd_soc_component *component;
for_each_card_components(card, component) { if (!strcmp(component->name, "i2c-INT343A:00")) { - dev_dbg(component->dev, "enabling jack detect for resume.\n"); rt286_mic_detect(component, &card_headset); break; } } + return 0; }
@@ -291,11 +269,10 @@ static int bdw_rt286_probe(struct platform_device *pdev) int ret;
bdw_rt286_card.dev = &pdev->dev; - /* override platform name, if required */ mach = pdev->dev.platform_data; - ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card, - mach->mach_params.platform); + + ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card, mach->mach_params.platform); if (ret) return ret;
Drop redundant and update valuable comments within the file to increase readability. This patch also revisits module information and kconfig help strings.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/Kconfig | 2 +- sound/soc/intel/boards/bdw_rt286.c | 23 +++++++---------------- 2 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index 817b4c04bf6a..aa12d7e3dd2f 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -85,7 +85,7 @@ config SND_SOC_INTEL_BDW_RT5677_MACH If unsure select "N".
config SND_SOC_INTEL_BROADWELL_MACH - tristate "Broadwell Wildcatpoint" + tristate "Broadwell with RT286 I2S codec" depends on I2C depends on I2C_DESIGNWARE_PLATFORM || COMPILE_TEST depends on X86_INTEL_LPSS || COMPILE_TEST diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index fbef19a545b0..51b37dafb469 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Intel Broadwell Wildcatpoint SST Audio + * Sound card driver for Intel Broadwell Wildcat Point with Realtek 286 * * Copyright (C) 2013, Intel Corporation. All rights reserved. */ @@ -16,7 +16,7 @@ #include "../../codecs/rt286.h"
static struct snd_soc_jack card_headset; -/* Headset jack detection DAPM pins */ + static struct snd_soc_jack_pin card_headset_pins[] = { { .pin = "Mic Jack", @@ -43,18 +43,14 @@ static const struct snd_soc_dapm_widget card_widgets[] = { };
static const struct snd_soc_dapm_route card_routes[] = { - /* speaker */ {"Speaker", NULL, "SPOR"}, {"Speaker", NULL, "SPOL"},
- /* HP jack connectors - unknown if we have jack deteck */ {"Headphone Jack", NULL, "HPO Pin"},
- /* other jacks */ {"MIC1", NULL, "Mic Jack"}, {"LINE1", NULL, "Line Jack"},
- /* digital mics */ {"DMIC1 Pin", NULL, "DMIC1"}, {"DMIC2 Pin", NULL, "DMIC2"},
@@ -84,10 +80,10 @@ static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
- /* The ADSP will covert the FE rate to 48k, stereo */ + /* The ADSP will convert the FE rate to 48kHz, stereo. */ rate->min = rate->max = 48000; channels->min = channels->max = 2; - /* set SSP0 to 16 bit */ + /* Set SSP0 to 16 bit. */ params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
return 0; @@ -147,7 +143,6 @@ SND_SOC_DAILINK_DEF(platform, DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audi SND_SOC_DAILINK_DEF(codec, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1"))); SND_SOC_DAILINK_DEF(ssp0_port, DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
-/* broadwell digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link card_dai_links[] = { /* Front End DAI links */ { @@ -240,14 +235,13 @@ static int card_resume_post(struct snd_soc_card *card) return 0; }
-/* use space before codec name to simplify card ID, and simplify driver name */ +/* Use space before codec name to simplify card ID, and simplify driver name. */ #define SOF_CARD_NAME "bdw rt286" /* card name will be 'sof-bdw rt286' */ #define SOF_DRIVER_NAME "SOF"
#define CARD_NAME "broadwell-rt286" #define DRIVER_NAME NULL /* card name will be used for driver name */
-/* broadwell audio machine driver for WPT + RT286S */ static struct snd_soc_card bdw_rt286_card = { .owner = THIS_MODULE, .dai_link = card_dai_links, @@ -269,14 +263,12 @@ static int bdw_rt286_probe(struct platform_device *pdev) int ret;
bdw_rt286_card.dev = &pdev->dev; - /* override platform name, if required */ mach = pdev->dev.platform_data;
ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card, mach->mach_params.platform); if (ret) return ret;
- /* set card and driver name */ if (snd_soc_acpi_sof_parent(&pdev->dev)) { bdw_rt286_card.name = SOF_CARD_NAME; bdw_rt286_card.driver_name = SOF_DRIVER_NAME; @@ -308,8 +300,7 @@ static struct platform_driver bdw_rt286_driver = {
module_platform_driver(bdw_rt286_driver)
-/* Module information */ MODULE_AUTHOR("Liam Girdwood, Xingchao Wang"); -MODULE_DESCRIPTION("Intel SST Audio for WPT/Broadwell"); -MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Sound card driver for Intel Broadwell Wildcat Point with Realtek 286"); +MODULE_LICENSE("GPL"); MODULE_ALIAS("platform:bdw_rt286");
Declare local 'dev' and make use of it plus dev_get_platdata() to improve code readability. Relocate few relevant to the function macros for the exact same read too.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/bdw_rt286.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index 51b37dafb469..d401b2c85b20 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -235,13 +235,6 @@ static int card_resume_post(struct snd_soc_card *card) return 0; }
-/* Use space before codec name to simplify card ID, and simplify driver name. */ -#define SOF_CARD_NAME "bdw rt286" /* card name will be 'sof-bdw rt286' */ -#define SOF_DRIVER_NAME "SOF" - -#define CARD_NAME "broadwell-rt286" -#define DRIVER_NAME NULL /* card name will be used for driver name */ - static struct snd_soc_card bdw_rt286_card = { .owner = THIS_MODULE, .dai_link = card_dai_links, @@ -257,27 +250,33 @@ static struct snd_soc_card bdw_rt286_card = { .resume_post = card_resume_post, };
+/* Use space before codec name to simplify card ID, and simplify driver name. */ +#define SOF_CARD_NAME "bdw rt286" /* card name will be 'sof-bdw rt286' */ +#define SOF_DRIVER_NAME "SOF" + +#define CARD_NAME "broadwell-rt286" + static int bdw_rt286_probe(struct platform_device *pdev) { struct snd_soc_acpi_mach *mach; + struct device *dev = &pdev->dev; int ret;
- bdw_rt286_card.dev = &pdev->dev; - mach = pdev->dev.platform_data; + bdw_rt286_card.dev = dev; + mach = dev_get_platdata(dev);
ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card, mach->mach_params.platform); if (ret) return ret;
- if (snd_soc_acpi_sof_parent(&pdev->dev)) { + if (snd_soc_acpi_sof_parent(dev)) { bdw_rt286_card.name = SOF_CARD_NAME; bdw_rt286_card.driver_name = SOF_DRIVER_NAME; } else { bdw_rt286_card.name = CARD_NAME; - bdw_rt286_card.driver_name = DRIVER_NAME; }
- return devm_snd_soc_register_card(&pdev->dev, &bdw_rt286_card); + return devm_snd_soc_register_card(dev, &bdw_rt286_card); }
static int bdw_rt286_remove(struct platform_device *pdev)
Print status if setting sysclk fails.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/bdw_rt286.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index d401b2c85b20..e173546b68cc 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -98,7 +98,7 @@ static int codec_link_hw_params(struct snd_pcm_substream *substream,
ret = snd_soc_dai_set_sysclk(codec_dai, RT286_SCLK_S_PLL, 24000000, SND_SOC_CLOCK_IN); if (ret < 0) { - dev_err(rtd->dev, "can't set codec sysclk configuration\n"); + dev_err(rtd->dev, "set codec sysclk failed: %d\n", ret); return ret; }
Drop redundant 'ret' assignemnt, stop ignoring set_jack() return value and reword local 'component' variable to 'codec' to improve readability.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/bdw_rt286.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index e173546b68cc..81da9781c3ba 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -61,8 +61,8 @@ static const struct snd_soc_dapm_route card_routes[] = {
static int codec_link_init(struct snd_soc_pcm_runtime *rtd) { - struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; - int ret = 0; + struct snd_soc_component *codec = asoc_rtd_to_codec(rtd, 0)->component; + int ret;
ret = snd_soc_card_jack_new_pins(rtd->card, "Headset", SND_JACK_HEADSET | SND_JACK_BTN_0, &card_headset, card_headset_pins, @@ -70,8 +70,7 @@ static int codec_link_init(struct snd_soc_pcm_runtime *rtd) if (ret) return ret;
- rt286_mic_detect(component, &card_headset); - return 0; + return rt286_mic_detect(codec, &card_headset); }
static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
Make use of card->remove() rather than pdev->remove() to unassign jack during card unbind procedure.
To reduce code size, define unified jack setter in form of bdw_rt286_set_jack() and invoke it during remove(), suspend_pre() and resume_port().
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/bdw_rt286.c | 50 +++++++++--------------------- 1 file changed, 14 insertions(+), 36 deletions(-)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index 81da9781c3ba..a2ff4da0bb93 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -199,43 +199,33 @@ static struct snd_soc_dai_link card_dai_links[] = { }, };
-static void broadwell_disable_jack(struct snd_soc_card *card) +static int card_set_jack(struct snd_soc_card *card, struct snd_soc_jack *jack) { - struct snd_soc_component *component; - - for_each_card_components(card, component) { - if (!strcmp(component->name, "i2c-INT343A:00")) { - dev_dbg(component->dev, "disabling jack detect before going to suspend.\n"); - rt286_mic_detect(component, NULL); - break; - } - } + struct snd_soc_dai *codec_dai = snd_soc_card_get_codec_dai(card, "rt286-aif1"); + + return rt286_mic_detect(codec_dai->component, jack); }
-static int card_suspend_pre(struct snd_soc_card *card) +static int card_remove(struct snd_soc_card *card) { - broadwell_disable_jack(card); + return card_set_jack(card, NULL); +}
- return 0; +static int card_suspend_pre(struct snd_soc_card *card) +{ + return card_set_jack(card, NULL); }
static int card_resume_post(struct snd_soc_card *card) { - struct snd_soc_component *component; - - for_each_card_components(card, component) { - if (!strcmp(component->name, "i2c-INT343A:00")) { - dev_dbg(component->dev, "enabling jack detect for resume.\n"); - rt286_mic_detect(component, &card_headset); - break; - } - } - - return 0; + return card_set_jack(card, &card_headset); }
static struct snd_soc_card bdw_rt286_card = { .owner = THIS_MODULE, + .remove = card_remove, + .suspend_pre = card_suspend_pre, + .resume_post = card_resume_post, .dai_link = card_dai_links, .num_links = ARRAY_SIZE(card_dai_links), .controls = card_controls, @@ -245,8 +235,6 @@ static struct snd_soc_card bdw_rt286_card = { .dapm_routes = card_routes, .num_dapm_routes = ARRAY_SIZE(card_routes), .fully_routed = true, - .suspend_pre = card_suspend_pre, - .resume_post = card_resume_post, };
/* Use space before codec name to simplify card ID, and simplify driver name. */ @@ -278,18 +266,8 @@ static int bdw_rt286_probe(struct platform_device *pdev) return devm_snd_soc_register_card(dev, &bdw_rt286_card); }
-static int bdw_rt286_remove(struct platform_device *pdev) -{ - struct snd_soc_card *card = platform_get_drvdata(pdev); - - broadwell_disable_jack(card); - - return 0; -} - static struct platform_driver bdw_rt286_driver = { .probe = bdw_rt286_probe, - .remove = bdw_rt286_remove, .driver = { .name = "bdw_rt286", .pm = &snd_soc_pm_ops
bdw_rt286_fe_ops is redundant as platform FE DAIs already limit the number of channels available for the endpoint.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/intel/boards/bdw_rt286.c | 25 ------------------------- 1 file changed, 25 deletions(-)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index a2ff4da0bb93..7d34e5002558 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -108,30 +108,6 @@ static const struct snd_soc_ops codec_link_ops = { .hw_params = codec_link_hw_params, };
-static const unsigned int channels[] = { - 2, -}; - -static const struct snd_pcm_hw_constraint_list constraints_channels = { - .count = ARRAY_SIZE(channels), - .list = channels, - .mask = 0, -}; - -static int bdw_rt286_fe_startup(struct snd_pcm_substream *substream) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - - /* Board supports stereo configuration only */ - runtime->hw.channels_max = 2; - return snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, - &constraints_channels); -} - -static const struct snd_soc_ops bdw_rt286_fe_ops = { - .startup = bdw_rt286_fe_startup, -}; - SND_SOC_DAILINK_DEF(system, DAILINK_COMP_ARRAY(COMP_CPU("System Pin"))); SND_SOC_DAILINK_DEF(offload0, DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin"))); SND_SOC_DAILINK_DEF(offload1, DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin"))); @@ -150,7 +126,6 @@ static struct snd_soc_dai_link card_dai_links[] = { .nonatomic = 1, .dynamic = 1, .trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST }, - .ops = &bdw_rt286_fe_ops, .dpcm_playback = 1, .dpcm_capture = 1, SND_SOC_DAILINK_REG(system, dummy, platform),
On 2022-06-10 2:36 PM, Cezary Rojewski wrote:
A number of patches improving overall quality and readability of haswell.c and broadwell.c source files found in sound/soc/intel/boards. Both files are first renamed and only then actual changes are being incrementally added. The respective names are: hsw_rt5640 and bdw_rt286 to match the pattern found in more recent boards.
Most patches bring no functional change - the more impactful patches at are placed the end:
Refactor of suspend/resume flow for the bdw_rt286 board by dropping dev->remove() in favour of card->remove() and adjust jack handling to reduce code size slightly by implementing card_set_jack().
The last patch is removing of FE DAI ops. Given the existence of platform FE DAI capabilities (either static declaration or through topology file), this code is redundant.
Hello,
While this patchset reorganizes and rewords code of two boards in question, module (kernel module) names are unchanged. Currently those two are:
- snd_soc_sst_haswell.ko - snd_soc_sst_broadwell.ko
My question is: Is it viable to reword these two?
Both modules accept no custom parameters, perhaps *dyndbg* is the only possibility so the impact is reduced.
Regards, Czarek
On 6/10/22 12:33, Cezary Rojewski wrote:
On 2022-06-10 2:36 PM, Cezary Rojewski wrote:
A number of patches improving overall quality and readability of haswell.c and broadwell.c source files found in sound/soc/intel/boards. Both files are first renamed and only then actual changes are being incrementally added. The respective names are: hsw_rt5640 and bdw_rt286 to match the pattern found in more recent boards.
Most patches bring no functional change - the more impactful patches at are placed the end:
Refactor of suspend/resume flow for the bdw_rt286 board by dropping dev->remove() in favour of card->remove() and adjust jack handling to reduce code size slightly by implementing card_set_jack().
The last patch is removing of FE DAI ops. Given the existence of platform FE DAI capabilities (either static declaration or through topology file), this code is redundant.
Hello,
While this patchset reorganizes and rewords code of two boards in question, module (kernel module) names are unchanged. Currently those two are:
- snd_soc_sst_haswell.ko
- snd_soc_sst_broadwell.ko
My question is: Is it viable to reword these two?
Both modules accept no custom parameters, perhaps *dyndbg* is the only possibility so the impact is reduced.
Thanks for asking the question.
I have no objection to the driver name change and haswell is not used in commercial products outside of Intel.
You have a point that most of the machine driver module names make limited sense in hindsight, but it's better to leave them as is. Changing them will increase confusion IMHO.
We have scripts to remove/re-insert modules and every time we add a name change we break the test suite. This happened when we changed all the PCI names, it wasn't pretty. See e.g. all the 'obsolete' references in those scripts to keep them working across kernel versions.
https://github.com/thesofproject/sof-test/blob/main/tools/kmod/sof_remove.sh...
we also enable dyndbg with /etc/modprobe.d/sof-dyndbg.conf deployed on test devices, if we change module names it gives everyone involved in CI/testing more work.
And last if you Google a bit you'll see references in a couple of wikis and bug reports to modprobe snd-soc-sst-broadwell, so if you change the module name you make the information obsolete.
On 2022-06-10 8:48 PM, Pierre-Louis Bossart wrote:
On 6/10/22 12:33, Cezary Rojewski wrote:
...
Hello,
While this patchset reorganizes and rewords code of two boards in question, module (kernel module) names are unchanged. Currently those two are:
- snd_soc_sst_haswell.ko
- snd_soc_sst_broadwell.ko
My question is: Is it viable to reword these two?
Both modules accept no custom parameters, perhaps *dyndbg* is the only possibility so the impact is reduced.
Thanks for asking the question.
I have no objection to the driver name change and haswell is not used in commercial products outside of Intel.
(save #1)
You have a point that most of the machine driver module names make limited sense in hindsight, but it's better to leave them as is. Changing them will increase confusion IMHO.
(save #2)
We have scripts to remove/re-insert modules and every time we add a name change we break the test suite. This happened when we changed all the PCI names, it wasn't pretty. See e.g. all the 'obsolete' references in those scripts to keep them working across kernel versions.
https://github.com/thesofproject/sof-test/blob/main/tools/kmod/sof_remove.sh...
we also enable dyndbg with /etc/modprobe.d/sof-dyndbg.conf deployed on test devices, if we change module names it gives everyone involved in CI/testing more work.
And last if you Google a bit you'll see references in a couple of wikis and bug reports to modprobe snd-soc-sst-broadwell, so if you change the module name you make the information obsolete.
Hello,
Very much appreciate the input. I admit that at first #1 made me think it's OK to change the name for the two but the later portion of the message (#2 and onward) made me think otherwise. Decided to not that change part of current series in v2, will send module renames as separate two patches either today or later this week - if snd_soc_sst_broadwell rename is not welcome, it won't get merged.
Regards, Czarek
participants (2)
-
Cezary Rojewski -
Pierre-Louis Bossart