linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] Refactor Vangogh acp5x machine driver
@ 2023-02-16 10:32 Lucas Tanure
  2023-02-16 10:32 ` [PATCH 1/9] ASoC: amd: vangogh: Remove unnecessary init function Lucas Tanure
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:32 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

Provide small fixes and refactor the code for easier insertion of a new
platform using the same acp5x machine driver.

Lucas Tanure (9):
  ASoC: amd: vangogh: Remove unnecessary init function
  ASoC: amd: vangogh: Update code indentation
  ASoC: amd: vangogh: use sizeof of variable instead of struct type
  ASoC: amd: vangogh: remove unnecessarily included headers
  ASoC: amd: vangogh: use for_each_rtd_components instead of for
  ASoC: amd: vangogh: Check Bit Clock rate before snd_soc_dai_set_pll
  ASoC: amd: vangogh: Move nau8821 and CPU side code up for future
    platform
  ASoC: amd: vangogh: Centralize strings definition
  ASoC: amd: vangogh: Include cs35l41 in structs names

 sound/soc/amd/vangogh/acp5x-mach.c | 313 +++++++++++++----------------
 1 file changed, 141 insertions(+), 172 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/9] ASoC: amd: vangogh: Remove unnecessary init function
  2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
@ 2023-02-16 10:32 ` Lucas Tanure
  2023-02-16 10:32 ` [PATCH 2/9] ASoC: amd: vangogh: Update code indentation Lucas Tanure
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:32 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

Remove empty acp5x_cs35l41_init function

Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index eebf2650ad27..5bd9418919a0 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -73,11 +73,6 @@ static int acp5x_8821_init(struct snd_soc_pcm_runtime *rtd)
 	return ret;
 }
 
-static int acp5x_cs35l41_init(struct snd_soc_pcm_runtime *rtd)
-{
-	return 0;
-}
-
 static const unsigned int rates[] = {
 	48000,
 };
@@ -258,7 +253,6 @@ static struct snd_soc_dai_link acp5x_dai[] = {
 		.dpcm_playback = 1,
 		.playback_only = 1,
 		.ops = &acp5x_cs35l41_play_ops,
-		.init = acp5x_cs35l41_init,
 		SND_SOC_DAILINK_REG(acp5x_bt, cs35l41, platform),
 	},
 };
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/9] ASoC: amd: vangogh: Update code indentation
  2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
  2023-02-16 10:32 ` [PATCH 1/9] ASoC: amd: vangogh: Remove unnecessary init function Lucas Tanure
@ 2023-02-16 10:32 ` Lucas Tanure
  2023-02-16 15:15   ` Mark Brown
  2023-02-16 10:32 ` [PATCH 3/9] ASoC: amd: vangogh: use sizeof of variable instead of struct type Lucas Tanure
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:32 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

Make use of 100 character limit and modify indentation so code is
easier to read.
While at it:
 - sort includes in alphabetical order
 - sort variables declarations by line length
 - use smaller variables names
 - remove unnecessary "struct snd_soc_card *card" lines
 - insert blank lines before return
 - align defines

Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 172 ++++++++++++-----------------
 1 file changed, 71 insertions(+), 101 deletions(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index 5bd9418919a0..2675fbff9f6f 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -5,34 +5,31 @@
  *
  * Copyright 2021 Advanced Micro Devices, Inc.
  */
-
-#include <sound/soc.h>
-#include <sound/soc-dapm.h>
-#include <linux/module.h>
-#include <linux/io.h>
-#include <sound/pcm.h>
-#include <sound/pcm_params.h>
-
-#include <sound/jack.h>
+#include <linux/acpi.h>
 #include <linux/clk.h>
+#include <linux/dmi.h>
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
+#include <linux/io.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
-#include <linux/acpi.h>
-#include <linux/dmi.h>
+#include <linux/module.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
 
 #include "../../codecs/nau8821.h"
 #include "../../codecs/cs35l41.h"
-
 #include "acp5x.h"
 
-#define DRV_NAME "acp5x_mach"
-#define DUAL_CHANNEL		2
-#define ACP5X_NUVOTON_CODEC_DAI	"nau8821-hifi"
-#define VG_JUPITER 1
-#define ACP5X_NUVOTON_BCLK 3072000
-#define ACP5X_NAU8821_FREQ_OUT 12288000
+#define DRV_NAME			"acp5x_mach"
+#define DUAL_CHANNEL			2
+#define ACP5X_NUVOTON_CODEC_DAI		"nau8821-hifi"
+#define VG_JUPITER			1
+#define ACP5X_NUVOTON_BCLK		3072000
+#define ACP5X_NAU8821_FREQ_OUT		12288000
 
 static unsigned long acp5x_machine_id;
 static struct snd_soc_jack vg_headset;
@@ -50,16 +47,14 @@ static struct snd_soc_jack_pin acp5x_nau8821_jack_pins[] = {
 
 static int acp5x_8821_init(struct snd_soc_pcm_runtime *rtd)
 {
+	struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component;
 	int ret;
-	struct snd_soc_card *card = rtd->card;
-	struct snd_soc_component *component =
-					asoc_rtd_to_codec(rtd, 0)->component;
 
 	/*
 	 * Headset buttons map to the google Reference headset.
 	 * These can be configured by userspace.
 	 */
-	ret = snd_soc_card_jack_new_pins(card, "Headset Jack",
+	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset Jack",
 					 SND_JACK_HEADSET | SND_JACK_BTN_0,
 					 &vg_headset, acp5x_nau8821_jack_pins,
 					 ARRAY_SIZE(acp5x_nau8821_jack_pins));
@@ -70,6 +65,7 @@ static int acp5x_8821_init(struct snd_soc_pcm_runtime *rtd)
 
 	snd_jack_set_key(vg_headset.jack, SND_JACK_BTN_0, KEY_MEDIA);
 	nau8821_enable_jack_detect(component, &vg_headset);
+
 	return ret;
 }
 
@@ -100,42 +96,35 @@ static struct snd_pcm_hw_constraint_list constraints_sample_bits = {
 	.count = ARRAY_SIZE(acp5x_nau8821_format),
 };
 
-static int acp5x_8821_startup(struct snd_pcm_substream *substream)
+static int acp5x_8821_startup(struct snd_pcm_substream *sub)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
-	struct snd_soc_card *card = rtd->card;
-	struct acp5x_platform_info *machine = snd_soc_card_get_drvdata(card);
+	struct snd_pcm_runtime *runtime = sub->runtime;
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(sub);
+	struct acp5x_platform_info *machine = snd_soc_card_get_drvdata(rtd->card);
 
 	machine->play_i2s_instance = I2S_SP_INSTANCE;
 	machine->cap_i2s_instance = I2S_SP_INSTANCE;
 
 	runtime->hw.channels_max = DUAL_CHANNEL;
-	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
-				   &constraints_channels);
-	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
-				   &constraints_rates);
-	snd_pcm_hw_constraint_list(substream->runtime, 0,
-				   SNDRV_PCM_HW_PARAM_SAMPLE_BITS,
+	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, &constraints_channels);
+	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &constraints_rates);
+	snd_pcm_hw_constraint_list(sub->runtime, 0, SNDRV_PCM_HW_PARAM_SAMPLE_BITS,
 				   &constraints_sample_bits);
+
 	return 0;
 }
 
-static int acp5x_nau8821_hw_params(struct snd_pcm_substream *substream,
-				   struct snd_pcm_hw_params *params)
+static int acp5x_nau8821_hw_params(struct snd_pcm_substream *sub, struct snd_pcm_hw_params *params)
 {
-	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(sub);
 	struct snd_soc_card *card = rtd->card;
-	struct snd_soc_dai *codec_dai =
-			snd_soc_card_get_codec_dai(card,
-						   ACP5X_NUVOTON_CODEC_DAI);
+	struct snd_soc_dai *dai = snd_soc_card_get_codec_dai(card, ACP5X_NUVOTON_CODEC_DAI);
 	int ret;
 
-	ret = snd_soc_dai_set_sysclk(codec_dai, NAU8821_CLK_FLL_BLK, 0,
-				     SND_SOC_CLOCK_IN);
+	ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_FLL_BLK, 0, SND_SOC_CLOCK_IN);
 	if (ret < 0)
 		dev_err(card->dev, "can't set FS clock %d\n", ret);
-	ret = snd_soc_dai_set_pll(codec_dai, 0, 0, snd_soc_params_to_bclk(params),
+	ret = snd_soc_dai_set_pll(dai, 0, 0, snd_soc_params_to_bclk(params),
 				  params_rate(params) * 256);
 	if (ret < 0)
 		dev_err(card->dev, "can't set FLL: %d\n", ret);
@@ -145,35 +134,32 @@ static int acp5x_nau8821_hw_params(struct snd_pcm_substream *substream,
 
 static int acp5x_cs35l41_startup(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
-	struct snd_soc_card *card = rtd->card;
-	struct acp5x_platform_info *machine = snd_soc_card_get_drvdata(card);
+	struct acp5x_platform_info *machine = snd_soc_card_get_drvdata(rtd->card);
+	struct snd_pcm_runtime *runtime = substream->runtime;
 
 	machine->play_i2s_instance = I2S_HS_INSTANCE;
 
 	runtime->hw.channels_max = DUAL_CHANNEL;
-	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
-				   &constraints_channels);
-	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
-				   &constraints_rates);
+	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, &constraints_channels);
+	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &constraints_rates);
+
 	return 0;
 }
 
-static int acp5x_cs35l41_hw_params(struct snd_pcm_substream *substream,
-				   struct snd_pcm_hw_params *params)
+static int acp5x_cs35l41_hw_params(struct snd_pcm_substream *sub, struct snd_pcm_hw_params *params)
 {
-	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
-	struct snd_soc_card *card = rtd->card;
-	struct snd_soc_dai *codec_dai;
-	int ret, i;
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(sub);
 	unsigned int num_codecs = rtd->dai_link->num_codecs;
+	struct snd_soc_card *card = rtd->card;
+	struct snd_soc_dai *dai;
 	unsigned int bclk_val;
+	int ret, i;
 
 	ret = 0;
 	for (i = 0; i < num_codecs; i++) {
-		codec_dai = asoc_rtd_to_codec(rtd, i);
-		if (strcmp(codec_dai->name, "cs35l41-pcm") == 0) {
+		dai = asoc_rtd_to_codec(rtd, i);
+		if (strcmp(dai->name, "cs35l41-pcm") == 0) {
 			switch (params_rate(params)) {
 			case 48000:
 				bclk_val = 1536000;
@@ -183,7 +169,7 @@ static int acp5x_cs35l41_hw_params(struct snd_pcm_substream *substream,
 					params_rate(params));
 				return -EINVAL;
 			}
-			ret = snd_soc_component_set_sysclk(codec_dai->component,
+			ret = snd_soc_component_set_sysclk(dai->component,
 							   0, 0, bclk_val, SND_SOC_CLOCK_IN);
 			if (ret < 0) {
 				dev_err(card->dev, "failed to set sysclk for CS35l41 dai\n");
@@ -216,29 +202,18 @@ static struct snd_soc_codec_conf cs35l41_conf[] = {
 	},
 };
 
-SND_SOC_DAILINK_DEF(acp5x_i2s,
-		    DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.0")));
-
-SND_SOC_DAILINK_DEF(acp5x_bt,
-		    DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.1")));
-
-SND_SOC_DAILINK_DEF(nau8821,
-		    DAILINK_COMP_ARRAY(COMP_CODEC("i2c-NVTN2020:00",
-						  "nau8821-hifi")));
-
-SND_SOC_DAILINK_DEF(cs35l41,
-		    DAILINK_COMP_ARRAY(COMP_CODEC("spi-VLV1776:00", "cs35l41-pcm"),
-				       COMP_CODEC("spi-VLV1776:01", "cs35l41-pcm")));
-
-SND_SOC_DAILINK_DEF(platform,
-		    DAILINK_COMP_ARRAY(COMP_PLATFORM("acp5x_i2s_dma.0")));
+SND_SOC_DAILINK_DEF(platform,  DAILINK_COMP_ARRAY(COMP_PLATFORM("acp5x_i2s_dma.0")));
+SND_SOC_DAILINK_DEF(acp5x_i2s, DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.0")));
+SND_SOC_DAILINK_DEF(acp5x_bt,  DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.1")));
+SND_SOC_DAILINK_DEF(nau8821,   DAILINK_COMP_ARRAY(COMP_CODEC("i2c-NVTN2020:00", "nau8821-hifi")));
+SND_SOC_DAILINK_DEF(cs35l41,   DAILINK_COMP_ARRAY(COMP_CODEC("spi-VLV1776:00", "cs35l41-pcm"),
+						  COMP_CODEC("spi-VLV1776:01", "cs35l41-pcm")));
 
 static struct snd_soc_dai_link acp5x_dai[] = {
 	{
 		.name = "acp5x-8821-play",
 		.stream_name = "Playback/Capture",
-		.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,
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
 		.ops = &acp5x_8821_ops,
@@ -248,8 +223,7 @@ static struct snd_soc_dai_link acp5x_dai[] = {
 	{
 		.name = "acp5x-CS35L41-Stereo",
 		.stream_name = "CS35L41 Stereo Playback",
-		.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,
 		.dpcm_playback = 1,
 		.playback_only = 1,
 		.ops = &acp5x_cs35l41_play_ops,
@@ -257,37 +231,34 @@ static struct snd_soc_dai_link acp5x_dai[] = {
 	},
 };
 
-static int platform_clock_control(struct snd_soc_dapm_widget *w,
-				  struct snd_kcontrol *k, int  event)
+static int platform_clock_control(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event)
 {
 	struct snd_soc_dapm_context *dapm = w->dapm;
 	struct snd_soc_card *card = dapm->card;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	int ret = 0;
 
-	codec_dai = snd_soc_card_get_codec_dai(card, ACP5X_NUVOTON_CODEC_DAI);
-	if (!codec_dai) {
+	dai = snd_soc_card_get_codec_dai(card, ACP5X_NUVOTON_CODEC_DAI);
+	if (!dai) {
 		dev_err(card->dev, "Codec dai not found\n");
 		return -EIO;
 	}
 
 	if (SND_SOC_DAPM_EVENT_OFF(event)) {
-		ret = snd_soc_dai_set_sysclk(codec_dai, NAU8821_CLK_INTERNAL,
-					     0, SND_SOC_CLOCK_IN);
+		ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_INTERNAL, 0, SND_SOC_CLOCK_IN);
 		if (ret < 0) {
 			dev_err(card->dev, "set sysclk err = %d\n", ret);
 			return -EIO;
 		}
 	} else {
-		ret = snd_soc_dai_set_sysclk(codec_dai, NAU8821_CLK_FLL_BLK, 0,
-					     SND_SOC_CLOCK_IN);
+		ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_FLL_BLK, 0, SND_SOC_CLOCK_IN);
 		if (ret < 0)
-			dev_err(codec_dai->dev, "can't set BLK clock %d\n", ret);
-		ret = snd_soc_dai_set_pll(codec_dai, 0, 0, ACP5X_NUVOTON_BCLK,
-					  ACP5X_NAU8821_FREQ_OUT);
+			dev_err(dai->dev, "can't set BLK clock %d\n", ret);
+		ret = snd_soc_dai_set_pll(dai, 0, 0, ACP5X_NUVOTON_BCLK, ACP5X_NAU8821_FREQ_OUT);
 		if (ret < 0)
-			dev_err(codec_dai->dev, "can't set FLL: %d\n", ret);
+			dev_err(dai->dev, "can't set FLL: %d\n", ret);
 	}
+
 	return ret;
 }
 
@@ -336,6 +307,7 @@ static struct snd_soc_card acp5x_card = {
 static int acp5x_vg_quirk_cb(const struct dmi_system_id *id)
 {
 	acp5x_machine_id = VG_JUPITER;
+
 	return 1;
 }
 
@@ -352,12 +324,12 @@ static const struct dmi_system_id acp5x_vg_quirk_table[] = {
 
 static int acp5x_probe(struct platform_device *pdev)
 {
-	int ret;
 	struct acp5x_platform_info *machine;
+	struct device *dev = &pdev->dev;
 	struct snd_soc_card *card;
+	int ret;
 
-	machine = devm_kzalloc(&pdev->dev, sizeof(struct acp5x_platform_info),
-			       GFP_KERNEL);
+	machine = devm_kzalloc(&pdev->dev, sizeof(struct acp5x_platform_info), GFP_KERNEL);
 	if (!machine)
 		return -ENOMEM;
 
@@ -365,20 +337,18 @@ static int acp5x_probe(struct platform_device *pdev)
 	switch (acp5x_machine_id) {
 	case VG_JUPITER:
 		card = &acp5x_card;
-		acp5x_card.dev = &pdev->dev;
 		break;
 	default:
 		return -ENODEV;
 	}
+	card->dev = dev;
 	platform_set_drvdata(pdev, card);
 	snd_soc_card_set_drvdata(card, machine);
 
-	ret = devm_snd_soc_register_card(&pdev->dev, card);
-	if (ret) {
-		return dev_err_probe(&pdev->dev, ret,
-				     "snd_soc_register_card(%s) failed\n",
-				     acp5x_card.name);
-	}
+	ret = devm_snd_soc_register_card(dev, card);
+	if (ret)
+		return dev_err_probe(dev, ret, "snd_soc_register_card(%s) failed\n", card->name);
+
 	return 0;
 }
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/9] ASoC: amd: vangogh: use sizeof of variable instead of struct type
  2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
  2023-02-16 10:32 ` [PATCH 1/9] ASoC: amd: vangogh: Remove unnecessary init function Lucas Tanure
  2023-02-16 10:32 ` [PATCH 2/9] ASoC: amd: vangogh: Update code indentation Lucas Tanure
@ 2023-02-16 10:32 ` Lucas Tanure
  2023-02-16 10:32 ` [PATCH 4/9] ASoC: amd: vangogh: remove unnecessarily included headers Lucas Tanure
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:32 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

Use sizeof(*machine) instead of sizeof(struct acp5x_platform_info)

There is a possibility of bug when variable type has changed but
corresponding struct passed to the sizeof has not.

Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index 2675fbff9f6f..ec67345bcda4 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -329,7 +329,7 @@ static int acp5x_probe(struct platform_device *pdev)
 	struct snd_soc_card *card;
 	int ret;
 
-	machine = devm_kzalloc(&pdev->dev, sizeof(struct acp5x_platform_info), GFP_KERNEL);
+	machine = devm_kzalloc(&pdev->dev, sizeof(*machine), GFP_KERNEL);
 	if (!machine)
 		return -ENOMEM;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/9] ASoC: amd: vangogh: remove unnecessarily included headers
  2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
                   ` (2 preceding siblings ...)
  2023-02-16 10:32 ` [PATCH 3/9] ASoC: amd: vangogh: use sizeof of variable instead of struct type Lucas Tanure
@ 2023-02-16 10:32 ` Lucas Tanure
  2023-02-16 10:32 ` [PATCH 5/9] ASoC: amd: vangogh: use for_each_rtd_components instead of for Lucas Tanure
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:32 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

Remove unused includes and replace <linux/input.h> by
<linux/input-event-codes.h>

Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index ec67345bcda4..841b79fa6af5 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -5,23 +5,19 @@
  *
  * Copyright 2021 Advanced Micro Devices, Inc.
  */
+
 #include <linux/acpi.h>
-#include <linux/clk.h>
 #include <linux/dmi.h>
-#include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
-#include <linux/io.h>
 #include <linux/i2c.h>
-#include <linux/input.h>
+#include <linux/input-event-codes.h>
 #include <linux/module.h>
 #include <sound/jack.h>
-#include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
 
 #include "../../codecs/nau8821.h"
-#include "../../codecs/cs35l41.h"
 #include "acp5x.h"
 
 #define DRV_NAME			"acp5x_mach"
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/9] ASoC: amd: vangogh: use for_each_rtd_components instead of for
  2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
                   ` (3 preceding siblings ...)
  2023-02-16 10:32 ` [PATCH 4/9] ASoC: amd: vangogh: remove unnecessarily included headers Lucas Tanure
@ 2023-02-16 10:32 ` Lucas Tanure
  2023-02-16 10:32 ` [PATCH 6/9] ASoC: amd: vangogh: Check Bit Clock rate before snd_soc_dai_set_pll Lucas Tanure
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:32 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

To iterate over components use for_each_rtd_components
And compare to component name, so asoc_rtd_to_codec and the dai code can
be removed

Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 40 +++++++++++++++---------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index 841b79fa6af5..e7a7558f70ae 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -146,35 +146,35 @@ static int acp5x_cs35l41_startup(struct snd_pcm_substream *substream)
 static int acp5x_cs35l41_hw_params(struct snd_pcm_substream *sub, struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(sub);
-	unsigned int num_codecs = rtd->dai_link->num_codecs;
-	struct snd_soc_card *card = rtd->card;
-	struct snd_soc_dai *dai;
-	unsigned int bclk_val;
+	unsigned int bclk, rate = params_rate(params);
+	struct snd_soc_component *comp;
 	int ret, i;
 
-	ret = 0;
-	for (i = 0; i < num_codecs; i++) {
-		dai = asoc_rtd_to_codec(rtd, i);
-		if (strcmp(dai->name, "cs35l41-pcm") == 0) {
-			switch (params_rate(params)) {
-			case 48000:
-				bclk_val = 1536000;
-				break;
-			default:
-				dev_err(card->dev, "Invalid Samplerate:0x%x\n",
-					params_rate(params));
+	switch (rate) {
+	case 48000:
+		bclk = 1536000;
+		break;
+	default:
+		bclk = 0;
+		break;
+	}
+
+	for_each_rtd_components(rtd, i, comp) {
+		if (!(strcmp(comp->name, "spi-VLV1776:00")) ||
+		    !(strcmp(comp->name, "spi-VLV1776:01"))) {
+			if (!bclk) {
+				dev_err(comp->dev, "Invalid sample rate: 0x%x\n", rate);
 				return -EINVAL;
 			}
-			ret = snd_soc_component_set_sysclk(dai->component,
-							   0, 0, bclk_val, SND_SOC_CLOCK_IN);
-			if (ret < 0) {
-				dev_err(card->dev, "failed to set sysclk for CS35l41 dai\n");
+			ret = snd_soc_component_set_sysclk(comp, 0, 0, bclk, SND_SOC_CLOCK_IN);
+			if (ret) {
+				dev_err(comp->dev, "failed to set SYSCLK: %d\n", ret);
 				return ret;
 			}
 		}
 	}
 
-	return ret;
+	return 0;
 }
 
 static const struct snd_soc_ops acp5x_8821_ops = {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/9] ASoC: amd: vangogh: Check Bit Clock rate before snd_soc_dai_set_pll
  2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
                   ` (4 preceding siblings ...)
  2023-02-16 10:32 ` [PATCH 5/9] ASoC: amd: vangogh: use for_each_rtd_components instead of for Lucas Tanure
@ 2023-02-16 10:32 ` Lucas Tanure
  2023-02-16 10:32 ` [PATCH 7/9] ASoC: amd: vangogh: Move nau8821 and CPU side code up for future platform Lucas Tanure
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:32 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

Check bit clock is valid before setting it with snd_soc_dai_set_pll

Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index e7a7558f70ae..7072352389ab 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -115,13 +115,19 @@ static int acp5x_nau8821_hw_params(struct snd_pcm_substream *sub, struct snd_pcm
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(sub);
 	struct snd_soc_card *card = rtd->card;
 	struct snd_soc_dai *dai = snd_soc_card_get_codec_dai(card, ACP5X_NUVOTON_CODEC_DAI);
-	int ret;
+	int ret, bclk;
 
 	ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_FLL_BLK, 0, SND_SOC_CLOCK_IN);
 	if (ret < 0)
 		dev_err(card->dev, "can't set FS clock %d\n", ret);
-	ret = snd_soc_dai_set_pll(dai, 0, 0, snd_soc_params_to_bclk(params),
-				  params_rate(params) * 256);
+
+	bclk = snd_soc_params_to_bclk(params);
+	if (bclk < 0) {
+		dev_err(dai->dev, "Fail to get BCLK rate: %d\n", bclk);
+		return bclk;
+	}
+
+	ret = snd_soc_dai_set_pll(dai, 0, 0, bclk, params_rate(params) * 256);
 	if (ret < 0)
 		dev_err(card->dev, "can't set FLL: %d\n", ret);
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/9] ASoC: amd: vangogh: Move nau8821 and CPU side code up for future platform
  2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
                   ` (5 preceding siblings ...)
  2023-02-16 10:32 ` [PATCH 6/9] ASoC: amd: vangogh: Check Bit Clock rate before snd_soc_dai_set_pll Lucas Tanure
@ 2023-02-16 10:32 ` Lucas Tanure
  2023-02-16 10:32 ` [PATCH 8/9] ASoC: amd: vangogh: Centralize strings definition Lucas Tanure
  2023-02-16 10:33 ` [PATCH 9/9] ASoC: amd: vangogh: Include cs35l41 in structs names Lucas Tanure
  8 siblings, 0 replies; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:32 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

Move nau8821 and CPU side code up in the source so future platforms can
be added.

Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 93 +++++++++++++++---------------
 1 file changed, 47 insertions(+), 46 deletions(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index 7072352389ab..2072151957a6 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -30,6 +30,11 @@
 static unsigned long acp5x_machine_id;
 static struct snd_soc_jack vg_headset;
 
+SND_SOC_DAILINK_DEF(platform,  DAILINK_COMP_ARRAY(COMP_PLATFORM("acp5x_i2s_dma.0")));
+SND_SOC_DAILINK_DEF(acp5x_i2s, DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.0")));
+SND_SOC_DAILINK_DEF(acp5x_bt,  DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.1")));
+SND_SOC_DAILINK_DEF(nau8821,   DAILINK_COMP_ARRAY(COMP_CODEC("i2c-NVTN2020:00", "nau8821-hifi")));
+
 static struct snd_soc_jack_pin acp5x_nau8821_jack_pins[] = {
 	{
 		.pin	= "Headphone",
@@ -134,6 +139,48 @@ static int acp5x_nau8821_hw_params(struct snd_pcm_substream *sub, struct snd_pcm
 	return ret;
 }
 
+static int platform_clock_control(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event)
+{
+	struct snd_soc_dapm_context *dapm = w->dapm;
+	struct snd_soc_card *card = dapm->card;
+	struct snd_soc_dai *dai;
+	int ret = 0;
+
+	dai = snd_soc_card_get_codec_dai(card, ACP5X_NUVOTON_CODEC_DAI);
+	if (!dai) {
+		dev_err(card->dev, "Codec dai not found\n");
+		return -EIO;
+	}
+
+	if (SND_SOC_DAPM_EVENT_OFF(event)) {
+		ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_INTERNAL, 0, SND_SOC_CLOCK_IN);
+		if (ret < 0) {
+			dev_err(card->dev, "set sysclk err = %d\n", ret);
+			return -EIO;
+		}
+	} else {
+		ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_FLL_BLK, 0, SND_SOC_CLOCK_IN);
+		if (ret < 0)
+			dev_err(dai->dev, "can't set BLK clock %d\n", ret);
+		ret = snd_soc_dai_set_pll(dai, 0, 0, ACP5X_NUVOTON_BCLK, ACP5X_NAU8821_FREQ_OUT);
+		if (ret < 0)
+			dev_err(dai->dev, "can't set FLL: %d\n", ret);
+	}
+
+	return ret;
+}
+
+static const struct snd_kcontrol_new acp5x_8821_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Headphone"),
+	SOC_DAPM_PIN_SWITCH("Headset Mic"),
+	SOC_DAPM_PIN_SWITCH("Int Mic"),
+};
+
+static const struct snd_soc_ops acp5x_8821_ops = {
+	.startup = acp5x_8821_startup,
+	.hw_params = acp5x_nau8821_hw_params,
+};
+
 static int acp5x_cs35l41_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
@@ -183,11 +230,6 @@ static int acp5x_cs35l41_hw_params(struct snd_pcm_substream *sub, struct snd_pcm
 	return 0;
 }
 
-static const struct snd_soc_ops acp5x_8821_ops = {
-	.startup = acp5x_8821_startup,
-	.hw_params = acp5x_nau8821_hw_params,
-};
-
 static const struct snd_soc_ops acp5x_cs35l41_play_ops = {
 	.startup = acp5x_cs35l41_startup,
 	.hw_params = acp5x_cs35l41_hw_params,
@@ -204,10 +246,6 @@ static struct snd_soc_codec_conf cs35l41_conf[] = {
 	},
 };
 
-SND_SOC_DAILINK_DEF(platform,  DAILINK_COMP_ARRAY(COMP_PLATFORM("acp5x_i2s_dma.0")));
-SND_SOC_DAILINK_DEF(acp5x_i2s, DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.0")));
-SND_SOC_DAILINK_DEF(acp5x_bt,  DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.1")));
-SND_SOC_DAILINK_DEF(nau8821,   DAILINK_COMP_ARRAY(COMP_CODEC("i2c-NVTN2020:00", "nau8821-hifi")));
 SND_SOC_DAILINK_DEF(cs35l41,   DAILINK_COMP_ARRAY(COMP_CODEC("spi-VLV1776:00", "cs35l41-pcm"),
 						  COMP_CODEC("spi-VLV1776:01", "cs35l41-pcm")));
 
@@ -233,43 +271,6 @@ static struct snd_soc_dai_link acp5x_dai[] = {
 	},
 };
 
-static int platform_clock_control(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event)
-{
-	struct snd_soc_dapm_context *dapm = w->dapm;
-	struct snd_soc_card *card = dapm->card;
-	struct snd_soc_dai *dai;
-	int ret = 0;
-
-	dai = snd_soc_card_get_codec_dai(card, ACP5X_NUVOTON_CODEC_DAI);
-	if (!dai) {
-		dev_err(card->dev, "Codec dai not found\n");
-		return -EIO;
-	}
-
-	if (SND_SOC_DAPM_EVENT_OFF(event)) {
-		ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_INTERNAL, 0, SND_SOC_CLOCK_IN);
-		if (ret < 0) {
-			dev_err(card->dev, "set sysclk err = %d\n", ret);
-			return -EIO;
-		}
-	} else {
-		ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_FLL_BLK, 0, SND_SOC_CLOCK_IN);
-		if (ret < 0)
-			dev_err(dai->dev, "can't set BLK clock %d\n", ret);
-		ret = snd_soc_dai_set_pll(dai, 0, 0, ACP5X_NUVOTON_BCLK, ACP5X_NAU8821_FREQ_OUT);
-		if (ret < 0)
-			dev_err(dai->dev, "can't set FLL: %d\n", ret);
-	}
-
-	return ret;
-}
-
-static const struct snd_kcontrol_new acp5x_8821_controls[] = {
-	SOC_DAPM_PIN_SWITCH("Headphone"),
-	SOC_DAPM_PIN_SWITCH("Headset Mic"),
-	SOC_DAPM_PIN_SWITCH("Int Mic"),
-};
-
 static const struct snd_soc_dapm_widget acp5x_8821_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone", NULL),
 	SND_SOC_DAPM_MIC("Headset Mic", NULL),
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 8/9] ASoC: amd: vangogh: Centralize strings definition
  2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
                   ` (6 preceding siblings ...)
  2023-02-16 10:32 ` [PATCH 7/9] ASoC: amd: vangogh: Move nau8821 and CPU side code up for future platform Lucas Tanure
@ 2023-02-16 10:32 ` Lucas Tanure
  2023-02-16 15:37   ` Mark Brown
  2023-02-16 10:33 ` [PATCH 9/9] ASoC: amd: vangogh: Include cs35l41 in structs names Lucas Tanure
  8 siblings, 1 reply; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:32 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

Replace occurrences of strings by their definition, avoiding bugs where
the string changed, but not all places have been modified

Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index 2072151957a6..74c7b267dcfd 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -22,10 +22,13 @@
 
 #define DRV_NAME			"acp5x_mach"
 #define DUAL_CHANNEL			2
-#define ACP5X_NUVOTON_CODEC_DAI		"nau8821-hifi"
 #define VG_JUPITER			1
-#define ACP5X_NUVOTON_BCLK		3072000
-#define ACP5X_NAU8821_FREQ_OUT		12288000
+#define NAU8821_BCLK			3072000
+#define NAU8821_FREQ_OUT		12288000
+#define NAU8821_DAI			"nau8821-hifi"
+#define CS35L41_LNAME			"spi-VLV1776:00"
+#define CS35L41_RNAME			"spi-VLV1776:01"
+#define CS35L41_DAI			"cs35l41-pcm"
 
 static unsigned long acp5x_machine_id;
 static struct snd_soc_jack vg_headset;
@@ -33,7 +36,7 @@ static struct snd_soc_jack vg_headset;
 SND_SOC_DAILINK_DEF(platform,  DAILINK_COMP_ARRAY(COMP_PLATFORM("acp5x_i2s_dma.0")));
 SND_SOC_DAILINK_DEF(acp5x_i2s, DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.0")));
 SND_SOC_DAILINK_DEF(acp5x_bt,  DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.1")));
-SND_SOC_DAILINK_DEF(nau8821,   DAILINK_COMP_ARRAY(COMP_CODEC("i2c-NVTN2020:00", "nau8821-hifi")));
+SND_SOC_DAILINK_DEF(nau8821,   DAILINK_COMP_ARRAY(COMP_CODEC("i2c-NVTN2020:00", NAU8821_DAI)));
 
 static struct snd_soc_jack_pin acp5x_nau8821_jack_pins[] = {
 	{
@@ -119,7 +122,7 @@ static int acp5x_nau8821_hw_params(struct snd_pcm_substream *sub, struct snd_pcm
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(sub);
 	struct snd_soc_card *card = rtd->card;
-	struct snd_soc_dai *dai = snd_soc_card_get_codec_dai(card, ACP5X_NUVOTON_CODEC_DAI);
+	struct snd_soc_dai *dai = snd_soc_card_get_codec_dai(card, NAU8821_DAI);
 	int ret, bclk;
 
 	ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_FLL_BLK, 0, SND_SOC_CLOCK_IN);
@@ -146,7 +149,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w, struct snd_kcon
 	struct snd_soc_dai *dai;
 	int ret = 0;
 
-	dai = snd_soc_card_get_codec_dai(card, ACP5X_NUVOTON_CODEC_DAI);
+	dai = snd_soc_card_get_codec_dai(card, NAU8821_DAI);
 	if (!dai) {
 		dev_err(card->dev, "Codec dai not found\n");
 		return -EIO;
@@ -162,7 +165,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w, struct snd_kcon
 		ret = snd_soc_dai_set_sysclk(dai, NAU8821_CLK_FLL_BLK, 0, SND_SOC_CLOCK_IN);
 		if (ret < 0)
 			dev_err(dai->dev, "can't set BLK clock %d\n", ret);
-		ret = snd_soc_dai_set_pll(dai, 0, 0, ACP5X_NUVOTON_BCLK, ACP5X_NAU8821_FREQ_OUT);
+		ret = snd_soc_dai_set_pll(dai, 0, 0, NAU8821_BCLK, NAU8821_FREQ_OUT);
 		if (ret < 0)
 			dev_err(dai->dev, "can't set FLL: %d\n", ret);
 	}
@@ -213,8 +216,7 @@ static int acp5x_cs35l41_hw_params(struct snd_pcm_substream *sub, struct snd_pcm
 	}
 
 	for_each_rtd_components(rtd, i, comp) {
-		if (!(strcmp(comp->name, "spi-VLV1776:00")) ||
-		    !(strcmp(comp->name, "spi-VLV1776:01"))) {
+		if (!(strcmp(comp->name, CS35L41_LNAME)) || !(strcmp(comp->name, CS35L41_RNAME))) {
 			if (!bclk) {
 				dev_err(comp->dev, "Invalid sample rate: 0x%x\n", rate);
 				return -EINVAL;
@@ -237,17 +239,17 @@ static const struct snd_soc_ops acp5x_cs35l41_play_ops = {
 
 static struct snd_soc_codec_conf cs35l41_conf[] = {
 	{
-		.dlc = COMP_CODEC_CONF("spi-VLV1776:00"),
+		.dlc = COMP_CODEC_CONF(CS35L41_LNAME),
 		.name_prefix = "Left",
 	},
 	{
-		.dlc = COMP_CODEC_CONF("spi-VLV1776:01"),
+		.dlc = COMP_CODEC_CONF(CS35L41_RNAME),
 		.name_prefix = "Right",
 	},
 };
 
-SND_SOC_DAILINK_DEF(cs35l41,   DAILINK_COMP_ARRAY(COMP_CODEC("spi-VLV1776:00", "cs35l41-pcm"),
-						  COMP_CODEC("spi-VLV1776:01", "cs35l41-pcm")));
+SND_SOC_DAILINK_DEF(cs35l41, DAILINK_COMP_ARRAY(COMP_CODEC(CS35L41_LNAME, CS35L41_DAI),
+						COMP_CODEC(CS35L41_RNAME, CS35L41_DAI)));
 
 static struct snd_soc_dai_link acp5x_dai[] = {
 	{
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 9/9] ASoC: amd: vangogh: Include cs35l41 in structs names
  2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
                   ` (7 preceding siblings ...)
  2023-02-16 10:32 ` [PATCH 8/9] ASoC: amd: vangogh: Centralize strings definition Lucas Tanure
@ 2023-02-16 10:33 ` Lucas Tanure
  8 siblings, 0 replies; 12+ messages in thread
From: Lucas Tanure @ 2023-02-16 10:33 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Vijendar Mukunda
  Cc: alsa-devel, linux-kernel, Lucas Tanure, kernel

Include cs35l41 in structs names so future platforms can be added and
reference the correct sound card

Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index 74c7b267dcfd..129cadcdf468 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -251,7 +251,7 @@ static struct snd_soc_codec_conf cs35l41_conf[] = {
 SND_SOC_DAILINK_DEF(cs35l41, DAILINK_COMP_ARRAY(COMP_CODEC(CS35L41_LNAME, CS35L41_DAI),
 						COMP_CODEC(CS35L41_RNAME, CS35L41_DAI)));
 
-static struct snd_soc_dai_link acp5x_dai[] = {
+static struct snd_soc_dai_link acp5x_8821_35l41_dai[] = {
 	{
 		.name = "acp5x-8821-play",
 		.stream_name = "Playback/Capture",
@@ -273,7 +273,7 @@ static struct snd_soc_dai_link acp5x_dai[] = {
 	},
 };
 
-static const struct snd_soc_dapm_widget acp5x_8821_widgets[] = {
+static const struct snd_soc_dapm_widget acp5x_8821_35l41_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone", NULL),
 	SND_SOC_DAPM_MIC("Headset Mic", NULL),
 	SND_SOC_DAPM_MIC("Int Mic", NULL),
@@ -281,7 +281,7 @@ static const struct snd_soc_dapm_widget acp5x_8821_widgets[] = {
 			    platform_clock_control, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 };
 
-static const struct snd_soc_dapm_route acp5x_8821_audio_route[] = {
+static const struct snd_soc_dapm_route acp5x_8821_35l41_audio_route[] = {
 	/* HP jack connectors - unknown if we have jack detection */
 	{ "Headphone", NULL, "HPOL" },
 	{ "Headphone", NULL, "HPOR" },
@@ -294,15 +294,15 @@ static const struct snd_soc_dapm_route acp5x_8821_audio_route[] = {
 	{ "Int Mic", NULL, "Platform Clock" },
 };
 
-static struct snd_soc_card acp5x_card = {
+static struct snd_soc_card acp5x_8821_35l41_card = {
 	.name = "acp5x",
 	.owner = THIS_MODULE,
-	.dai_link = acp5x_dai,
-	.num_links = ARRAY_SIZE(acp5x_dai),
-	.dapm_widgets = acp5x_8821_widgets,
-	.num_dapm_widgets = ARRAY_SIZE(acp5x_8821_widgets),
-	.dapm_routes = acp5x_8821_audio_route,
-	.num_dapm_routes = ARRAY_SIZE(acp5x_8821_audio_route),
+	.dai_link = acp5x_8821_35l41_dai,
+	.num_links = ARRAY_SIZE(acp5x_8821_35l41_dai),
+	.dapm_widgets = acp5x_8821_35l41_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(acp5x_8821_35l41_widgets),
+	.dapm_routes = acp5x_8821_35l41_audio_route,
+	.num_dapm_routes = ARRAY_SIZE(acp5x_8821_35l41_audio_route),
 	.codec_conf = cs35l41_conf,
 	.num_configs = ARRAY_SIZE(cs35l41_conf),
 	.controls = acp5x_8821_controls,
@@ -341,7 +341,7 @@ static int acp5x_probe(struct platform_device *pdev)
 	dmi_check_system(acp5x_vg_quirk_table);
 	switch (acp5x_machine_id) {
 	case VG_JUPITER:
-		card = &acp5x_card;
+		card = &acp5x_8821_35l41_card;
 		break;
 	default:
 		return -ENODEV;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/9] ASoC: amd: vangogh: Update code indentation
  2023-02-16 10:32 ` [PATCH 2/9] ASoC: amd: vangogh: Update code indentation Lucas Tanure
@ 2023-02-16 15:15   ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2023-02-16 15:15 UTC (permalink / raw)
  To: Lucas Tanure
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Vijendar Mukunda,
	alsa-devel, linux-kernel, kernel

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]

On Thu, Feb 16, 2023 at 10:32:53AM +0000, Lucas Tanure wrote:
> Make use of 100 character limit and modify indentation so code is
> easier to read.

I'm having a hard time seeing this as a helpful 

> While at it:
>  - sort includes in alphabetical order
>  - sort variables declarations by line length
>  - use smaller variables names
>  - remove unnecessary "struct snd_soc_card *card" lines
>  - insert blank lines before return
>  - align defines

This isn't helping make things easier to review :/

> -static int acp5x_8821_startup(struct snd_pcm_substream *substream)
> +static int acp5x_8821_startup(struct snd_pcm_substream *sub)

We do usually refer to substreams as such, I'm not sure this is
really helping.

> -	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
> -				   &constraints_channels);
> +	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, &constraints_channels);

I'm having a *really* hard time seeing this as helping with
legibility, it just makes things worse when viewed at 80 columns
but it's hard to see what it helps.  The 100 column limit is
flexibility we can use to avoid contortions but this is fairly
natural.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 8/9] ASoC: amd: vangogh: Centralize strings definition
  2023-02-16 10:32 ` [PATCH 8/9] ASoC: amd: vangogh: Centralize strings definition Lucas Tanure
@ 2023-02-16 15:37   ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2023-02-16 15:37 UTC (permalink / raw)
  To: Lucas Tanure
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Vijendar Mukunda,
	alsa-devel, linux-kernel, kernel

[-- Attachment #1: Type: text/plain, Size: 968 bytes --]

On Thu, Feb 16, 2023 at 10:32:59AM +0000, Lucas Tanure wrote:

> Replace occurrences of strings by their definition, avoiding bugs where
> the string changed, but not all places have been modified

>  #define DRV_NAME			"acp5x_mach"
>  #define DUAL_CHANNEL			2
> -#define ACP5X_NUVOTON_CODEC_DAI		"nau8821-hifi"
>  #define VG_JUPITER			1
> -#define ACP5X_NUVOTON_BCLK		3072000
> -#define ACP5X_NAU8821_FREQ_OUT		12288000
> +#define NAU8821_BCLK			3072000
> +#define NAU8821_FREQ_OUT		12288000
> +#define NAU8821_DAI			"nau8821-hifi"
> +#define CS35L41_LNAME			"spi-VLV1776:00"
> +#define CS35L41_RNAME			"spi-VLV1776:01"
> +#define CS35L41_DAI			"cs35l41-pcm"

These changes don't obviously correspond to the description of
the patch.  It looks like there's at least some renaming and
reindentation of things not related to DAI names here.  TBH I'm
not sure the removal of namespacing is a good idea, it's probably
not *super* likely but we might run into collisions.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-02-16 15:37 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-16 10:32 [PATCH 0/9] Refactor Vangogh acp5x machine driver Lucas Tanure
2023-02-16 10:32 ` [PATCH 1/9] ASoC: amd: vangogh: Remove unnecessary init function Lucas Tanure
2023-02-16 10:32 ` [PATCH 2/9] ASoC: amd: vangogh: Update code indentation Lucas Tanure
2023-02-16 15:15   ` Mark Brown
2023-02-16 10:32 ` [PATCH 3/9] ASoC: amd: vangogh: use sizeof of variable instead of struct type Lucas Tanure
2023-02-16 10:32 ` [PATCH 4/9] ASoC: amd: vangogh: remove unnecessarily included headers Lucas Tanure
2023-02-16 10:32 ` [PATCH 5/9] ASoC: amd: vangogh: use for_each_rtd_components instead of for Lucas Tanure
2023-02-16 10:32 ` [PATCH 6/9] ASoC: amd: vangogh: Check Bit Clock rate before snd_soc_dai_set_pll Lucas Tanure
2023-02-16 10:32 ` [PATCH 7/9] ASoC: amd: vangogh: Move nau8821 and CPU side code up for future platform Lucas Tanure
2023-02-16 10:32 ` [PATCH 8/9] ASoC: amd: vangogh: Centralize strings definition Lucas Tanure
2023-02-16 15:37   ` Mark Brown
2023-02-16 10:33 ` [PATCH 9/9] ASoC: amd: vangogh: Include cs35l41 in structs names Lucas Tanure

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).