Skip to contentSkip to conversation

ASoC: max9867: Driver update

PATCH In review Ladislav Michl 2 participants 19 messages Mar 1, 2018 – Mar 2, 2018

Subsystems (2)
Sound ASoC
lore
Share & tools
lore

Ladislav Michl

author
#0 lore raw

cover ASoC: max9867: Driver update

Hi,

here are first few patches for max9867 codec driver. It is still not enough to make it work (at least for me :)), but lets send cleanups first.

I would be happy to read clarification about using continuous rate - see patch 6.

Thank you,
ladis

Ladislav Michl (6):
ASoC: max9867: Show Kconfig entry
ASoC: max9867: Improve error logging
ASoC: max9867: Drop probe function
ASoC: max9867: Fix codec capabilities
ASoC: max9867: DSP mode
ASoC: max9867: Use continuous sample rate

 sound/soc/codecs/Kconfig   |   3 +-
 sound/soc/codecs/max9867.c | 122 ++++++++++++---------------------------------
 sound/soc/codecs/max9867.h |   1 -
 3 files changed, 34 insertions(+), 92 deletions(-)
Signature · 1 line
2.16.2
author
#1 lore raw

PATCH 1/6 ASoC: max9867: Show Kconfig entry

Allow codec to be selected.

Signed-off-by Ladislav Michl <[email protected]>
---
 sound/soc/codecs/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
sound/soc/codecs/Kconfig +2 -1
index d874706a493e..822df8d3d4f9 100644
@@ -640,7 +640,8 @@ config SND_SOC_MAX98504
 	depends on I2C
 
 config SND_SOC_MAX9867
-	tristate
+	tristate "Maxim MAX9867 CODEC"
+	depends on I2C
 
 config SND_SOC_MAX98925
        tristate
Signature · 1 line
2.16.2
author
#2 lore raw

PATCH 2/6 ASoC: max9867: Improve error logging

In reply to Ladislav Michl

Tell user what are clock rate limits and reindent log messages.

Signed-off-by Ladislav Michl <[email protected]>
---
 sound/soc/codecs/max9867.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
sound/soc/codecs/max9867.c +4 -3
index a4e8fafb7e6d..99b8035d2fda 100644
@@ -291,7 +291,9 @@ static int max9867_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 		value |= MAX9867_PSCLK_40_60;
 		max9867->pclk =  freq/4;
 	} else {
-		pr_err("bad clock frequency %d", freq);
+		dev_err(component->dev,
+			"Invalid clock frequency %uHz (required 10-60MHz)\n",
+			freq);
 		return -EINVAL;
 	}
 	value = value << MAX9867_PSCLK_SHIFT;

@@ -488,8 +490,7 @@ static int max9867_i2c_probe(struct i2c_client *i2c,
 	max9867->regmap = devm_regmap_init_i2c(i2c, &max9867_regmap);
 	if (IS_ERR(max9867->regmap)) {
 		ret = PTR_ERR(max9867->regmap);
-		dev_err(&i2c->dev,
-				"Failed to allocate regmap: %d\n", ret);
+		dev_err(&i2c->dev, "Failed to allocate regmap: %d\n", ret);
 		return ret;
 	}
 	ret = regmap_read(max9867->regmap,
Signature · 1 line
2.16.2
maintainer
#3 lore raw

Applied "ASoC: max9867: Improve error logging" to the asoc tree

The patch

ASoC: max9867: Improve error logging

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 8b9c716aada77e72e2fe82320aba5a95b75ca400 Mon Sep 17 00:00:00 2001

From: Ladislav Michl <[email protected]>
Date: Thu, 1 Mar 2018 15:19:53 +0100
Subject: [PATCH] ASoC: max9867: Improve error logging

Tell user what are clock rate limits and reindent log messages.

Signed-off-by Ladislav Michl <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/codecs/max9867.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
sound/soc/codecs/max9867.c +4 -3
index 2f60924fe919..ac30213deac4 100644
@@ -291,7 +291,9 @@ static int max9867_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 		value |= MAX9867_PSCLK_40_60;
 		max9867->pclk =  freq/4;
 	} else {
-		pr_err("bad clock frequency %d", freq);
+		dev_err(component->dev,
+			"Invalid clock frequency %uHz (required 10-60MHz)\n",
+			freq);
 		return -EINVAL;
 	}
 	value = value << MAX9867_PSCLK_SHIFT;

@@ -486,8 +488,7 @@ static int max9867_i2c_probe(struct i2c_client *i2c,
 	max9867->regmap = devm_regmap_init_i2c(i2c, &max9867_regmap);
 	if (IS_ERR(max9867->regmap)) {
 		ret = PTR_ERR(max9867->regmap);
-		dev_err(&i2c->dev,
-				"Failed to allocate regmap: %d\n", ret);
+		dev_err(&i2c->dev, "Failed to allocate regmap: %d\n", ret);
 		return ret;
 	}
 	ret = regmap_read(max9867->regmap,
Signature · 1 line
2.16.2
author
#4 lore raw

PATCH 3/6 ASoC: max9867: Drop probe function

In reply to Ladislav Michl

Driver probe function has no use and can be deleted.

Signed-off-by Ladislav Michl <[email protected]>
---
 sound/soc/codecs/max9867.c | 10 ----------
 sound/soc/codecs/max9867.h |  1 -
 2 files changed, 11 deletions(-)
sound/soc/codecs/max9867.c +0 -10
index 99b8035d2fda..3620b38291f0 100644
@@ -406,17 +406,7 @@ static int max9867_resume(struct device *dev)
 }
 #endif
 
-static int max9867_probe(struct snd_soc_component *component)
-{
-	struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component);
-
-	dev_dbg(component->dev, "max98090_probe\n");
-	max9867->component = component;
-	return 0;
-}
-
 static const struct snd_soc_component_driver max9867_component = {
-	.probe			= max9867_probe,
 	.controls		= max9867_snd_controls,
 	.num_controls		= ARRAY_SIZE(max9867_snd_controls),
 	.dapm_routes		= max9867_audio_map,
sound/soc/codecs/max9867.h +0 -1
index c0aea3d1f2ba..55cd9976ff47 100644
@@ -75,7 +75,6 @@
 /* codec private data */
 struct max9867_priv {
 	struct regmap *regmap;
-	struct snd_soc_component *component;
 	unsigned int sysclk;
 	unsigned int pclk;
 	unsigned int master;
Signature · 1 line
2.16.2
maintainer
#5 lore raw

Re: [PATCH 3/6] ASoC: max9867: Drop probe function

Quoted 1 line · Ladislav Michl

Driver probe function has no use and can be deleted.

This doesn't apply against current code, please check and resend.

author
#6 lore raw

Re: [PATCH 3/6] ASoC: max9867: Drop probe function

Quoted 4 lines · Mark Brown
Quoted 1 line · Ladislav Michl

Driver probe function has no use and can be deleted.

This doesn't apply against current code, please check and resend.

It was generated and tested against next-20180301. Which version do you want me to generate against? Based on 4.16-rc1 it'll fail to merge later.

Thank you,
ladis

maintainer
#7 lore raw

Re: [PATCH 3/6] ASoC: max9867: Drop probe function

Quoted 7 lines · Ladislav Michl
Quoted 1 line · Mark Brown

This doesn't apply against current code, please check and resend.

It was generated and tested against next-20180301. Which version do you want me to generate against? Based on 4.16-rc1 it'll fail to merge later.

That is my current tree but I don't have a topic for this device other than your patches... please resend and I'll take a look what git didn't like.

author
#8 lore raw

Re: [PATCH 3/6] ASoC: max9867: Drop probe function

Quoted 5 lines · Mark Brown
Quoted 3 lines · Ladislav Michl
Quoted 1 line · Mark Brown

This doesn't apply against current code, please check and resend.

Quoted 7 lines
Quoted 3 lines

It was generated and tested against next-20180301. Which version do you want me to generate against? Based on 4.16-rc1 it'll fail to merge later.

That is my current tree but I don't have a topic for this device other than your patches... please resend and I'll take a look what git didn't like.

Well, in fact you have, but it is misnamed as topic/wm9867. So I guess once you merge these two topic branches, things will get fixed.

Thank you,
ladis

maintainer
#9 lore raw

Re: [PATCH 3/6] ASoC: max9867: Drop probe function

Quoted 10 lines · Ladislav Michl
Quoted 8 lines · Mark Brown
Quoted 7 lines · Ladislav Michl
Quoted 1 line · Mark Brown

This doesn't apply against current code, please check and resend.

It was generated and tested against next-20180301. Which version do you want me to generate against? Based on 4.16-rc1 it'll fail to merge later.

Quoted 6 lines
Quoted 3 lines

That is my current tree but I don't have a topic for this device other than your patches... please resend and I'll take a look what git didn't like.

Well, in fact you have, but it is misnamed as topic/wm9867. So I guess once you merge these two topic branches, things will get fixed.

Sorry, meant to reply here: as I said above, please resend.

maintainer
#10 lore raw

Applied "ASoC: max9867: Drop probe function" to the asoc tree

In reply to Ladislav Michl

The patch

ASoC: max9867: Drop probe function

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From b3e9c3f0d98135d8b9991c2a34e42627c20664a4 Mon Sep 17 00:00:00 2001

From: Ladislav Michl <[email protected]>
Date: Fri, 2 Mar 2018 14:11:00 +0100
Subject: [PATCH] ASoC: max9867: Drop probe function

Driver probe function has no use and can be deleted.

Signed-off-by Ladislav Michl <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/codecs/max9867.c | 10 ----------
 sound/soc/codecs/max9867.h |  1 -
 2 files changed, 11 deletions(-)
sound/soc/codecs/max9867.c +0 -10
index 349d9cc116ee..4ea3287162ad 100644
@@ -413,17 +413,7 @@ static int max9867_resume(struct device *dev)
 }
 #endif
 
-static int max9867_probe(struct snd_soc_component *component)
-{
-	struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component);
-
-	dev_dbg(component->dev, "max98090_probe\n");
-	max9867->component = component;
-	return 0;
-}
-
 static const struct snd_soc_component_driver max9867_component = {
-	.probe			= max9867_probe,
 	.controls		= max9867_snd_controls,
 	.num_controls		= ARRAY_SIZE(max9867_snd_controls),
 	.dapm_routes		= max9867_audio_map,
sound/soc/codecs/max9867.h +0 -1
index c0aea3d1f2ba..55cd9976ff47 100644
@@ -75,7 +75,6 @@
 /* codec private data */
 struct max9867_priv {
 	struct regmap *regmap;
-	struct snd_soc_component *component;
 	unsigned int sysclk;
 	unsigned int pclk;
 	unsigned int master;
Signature · 1 line
2.16.2
author
#11 lore raw

PATCH 4/6 ASoC: max9867: Fix codec capabilities

In reply to Ladislav Michl

Codes is stereo only with playback and capture streams bind to the same rate.

Signed-off-by Ladislav Michl <[email protected]>
---
 sound/soc/codecs/max9867.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
sound/soc/codecs/max9867.c +3 -2
index 3620b38291f0..4160523ea720 100644
@@ -369,19 +369,20 @@ static struct snd_soc_dai_driver max9867_dai[] = {
 	.name = "max9867-aif1",
 	.playback = {
 		.stream_name = "HiFi Playback",
-		.channels_min = 1,
+		.channels_min = 2,
 		.channels_max = 2,
 		.rates = MAX9867_RATES,
 		.formats = MAX9867_FORMATS,
 	},
 	.capture = {
 		.stream_name = "HiFi Capture",
-		.channels_min = 1,
+		.channels_min = 2,
 		.channels_max = 2,
 		.rates = MAX9867_RATES,
 		.formats = MAX9867_FORMATS,
 	},
 	.ops = &max9867_dai_ops,
+	.symmetric_rates = 1,
 	}
 };
Signature · 1 line
2.16.2
maintainer
#12 lore raw

Applied "ASoC: max9867: Fix codec capabilities" to the asoc tree

The patch

ASoC: max9867: Fix codec capabilities

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From e6ceb922ca802711db4b48639a9c73f9dc412f87 Mon Sep 17 00:00:00 2001

From: Ladislav Michl <[email protected]>
Date: Thu, 1 Mar 2018 15:20:56 +0100
Subject: [PATCH] ASoC: max9867: Fix codec capabilities

Codes is stereo only with playback and capture streams bind to the same rate.

Signed-off-by Ladislav Michl <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/codecs/max9867.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
sound/soc/codecs/max9867.c +3 -2
index ac30213deac4..367c0329d5c8 100644
@@ -369,19 +369,20 @@ static struct snd_soc_dai_driver max9867_dai[] = {
 	.name = "max9867-aif1",
 	.playback = {
 		.stream_name = "HiFi Playback",
-		.channels_min = 1,
+		.channels_min = 2,
 		.channels_max = 2,
 		.rates = MAX9867_RATES,
 		.formats = MAX9867_FORMATS,
 	},
 	.capture = {
 		.stream_name = "HiFi Capture",
-		.channels_min = 1,
+		.channels_min = 2,
 		.channels_max = 2,
 		.rates = MAX9867_RATES,
 		.formats = MAX9867_FORMATS,
 	},
 	.ops = &max9867_dai_ops,
+	.symmetric_rates = 1,
 	}
 };
Signature · 1 line
2.16.2
author
#13 lore raw

PATCH 5/6 ASoC: max9867: DSP mode

In reply to Ladislav Michl

Add configuration for DSP mode.

Signed-off-by Ladislav Michl <[email protected]>
---
 sound/soc/codecs/max9867.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
sound/soc/codecs/max9867.c +10 -4
index 4160523ea720..4ea3287162ad 100644
@@ -325,10 +325,16 @@ static int max9867_dai_set_fmt(struct snd_soc_dai *codec_dai,
 		return -EINVAL;
 	}
 
-	/* for i2s compatible mode */
-	iface1A |= MAX9867_I2S_DLY;
-	/* SDOUT goes to hiz state after all data is transferred */
-	iface1A |= MAX9867_SDOUT_HIZ;
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		iface1A |= MAX9867_I2S_DLY;
+		break;
+	case SND_SOC_DAIFMT_DSP_A:
+		iface1A |= MAX9867_TDM_MODE | MAX9867_SDOUT_HIZ;
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	/* Clock inversion bits, BCI and WCI */
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
Signature · 1 line
2.16.2
maintainer
#14 lore raw

Applied "ASoC: max9867: DSP mode" to the asoc tree

The patch

ASoC: max9867: DSP mode

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 79e139744508105bd749300f364563934bff23f2 Mon Sep 17 00:00:00 2001

From: Ladislav Michl <[email protected]>
Date: Thu, 1 Mar 2018 15:21:30 +0100
Subject: [PATCH] ASoC: max9867: DSP mode

Add configuration for DSP mode.

Signed-off-by Ladislav Michl <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/codecs/max9867.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
sound/soc/codecs/max9867.c +10 -4
index 367c0329d5c8..0dbf3136e3ef 100644
@@ -325,10 +325,16 @@ static int max9867_dai_set_fmt(struct snd_soc_dai *codec_dai,
 		return -EINVAL;
 	}
 
-	/* for i2s compatible mode */
-	iface1A |= MAX9867_I2S_DLY;
-	/* SDOUT goes to hiz state after all data is transferred */
-	iface1A |= MAX9867_SDOUT_HIZ;
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		iface1A |= MAX9867_I2S_DLY;
+		break;
+	case SND_SOC_DAIFMT_DSP_A:
+		iface1A |= MAX9867_TDM_MODE | MAX9867_SDOUT_HIZ;
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	/* Clock inversion bits, BCI and WCI */
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
Signature · 1 line
2.16.2
author
#15 lore raw

PATCH 6/6 ASoC: max9867: Use continuous sample rate

In reply to Ladislav Michl

Drop "Common NI Values Table" and calculate LRCLK divider.

Signed-off-by Ladislav Michl <[email protected]>

---

Hi,

what is exact meaning of SNDRV_PCM_RATE_CONTINUOUS? What if codec is able to set "any" rate, but there are rounding errors? Shall we pick exact matches based on master clock frequency? Few other drivers are also setting SNDRV_PCM_RATE_CONTINUOUS, but certainly cannot set any rate exactly.

Thank you,
ladis

 sound/soc/codecs/max9867.c | 86 ++++++++--------------------------------------
 1 file changed, 15 insertions(+), 71 deletions(-)
sound/soc/codecs/max9867.c +15 -71
index 4ea3287162ad..3b552b417d0d 100644
@@ -126,75 +126,19 @@ static const struct snd_soc_dapm_route max9867_audio_map[] = {
 	{"LINE_IN", NULL, "Right Line"},
 };
 
-enum rates {
-	pcm_rate_8, pcm_rate_16, pcm_rate_24,
-	pcm_rate_32, pcm_rate_44,
-	pcm_rate_48, max_pcm_rate,
-};
-
-static const struct ni_div_rates {
-	u32 mclk;
-	u16 ni[max_pcm_rate];
-} ni_div[] = {
-	{11289600, {0x116A, 0x22D4, 0x343F, 0x45A9, 0x6000, 0x687D} },
-	{12000000, {0x1062, 0x20C5, 0x3127, 0x4189, 0x5A51, 0x624E} },
-	{12288000, {0x1000, 0x2000, 0x3000, 0x4000, 0x5833, 0x6000} },
-	{13000000, {0x0F20, 0x1E3F, 0x2D5F, 0x3C7F, 0x535F, 0x5ABE} },
-	{19200000, {0x0A3D, 0x147B, 0x1EB8, 0x28F6, 0x3873, 0x3D71} },
-	{24000000, {0x1062, 0x20C5, 0x1893, 0x4189, 0x5A51, 0x624E} },
-	{26000000, {0x0F20, 0x1E3F, 0x16AF, 0x3C7F, 0x535F, 0x5ABE} },
-	{27000000, {0x0E90, 0x1D21, 0x15D8, 0x3A41, 0x5048, 0x5762} },
-};
-
-static inline int get_ni_value(int mclk, int rate)
-{
-	int i, ret = 0;
-
-	/* find the closest rate index*/
-	for (i = 0; i < ARRAY_SIZE(ni_div); i++) {
-		if (ni_div[i].mclk >= mclk)
-			break;
-	}
-	if (i == ARRAY_SIZE(ni_div))
-		return -EINVAL;
-
-	switch (rate) {
-	case 8000:
-		return ni_div[i].ni[pcm_rate_8];
-	case 16000:
-		return ni_div[i].ni[pcm_rate_16];
-	case 32000:
-		return ni_div[i].ni[pcm_rate_32];
-	case 44100:
-		return ni_div[i].ni[pcm_rate_44];
-	case 48000:
-		return ni_div[i].ni[pcm_rate_48];
-	default:
-		pr_err("%s wrong rate %d\n", __func__, rate);
-		ret = -EINVAL;
-	}
-	return ret;
-}
-
 static int max9867_dai_hw_params(struct snd_pcm_substream *substream,
 		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
 {
 	struct snd_soc_component *component = dai->component;
 	struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component);
-	unsigned int ni_h, ni_l;
-	int value;
+	unsigned int ni = DIV_ROUND_CLOSEST_ULL(96ULL * 0x10000 * params_rate(params),
+						max9867->pclk);
 
-	value = get_ni_value(max9867->sysclk, params_rate(params));
-	if (value < 0)
-		return value;
-
-	ni_h = (0xFF00 & value) >> 8;
-	ni_l = 0x00FF & value;
 	/* set up the ni value */
 	regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKHIGH,
-		MAX9867_NI_HIGH_MASK, ni_h);
+		MAX9867_NI_HIGH_MASK, (0xFF00 & ni) >> 8);
 	regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKLOW,
-		MAX9867_NI_LOW_MASK, ni_l);
+		MAX9867_NI_LOW_MASK, 0x00FF & ni);
 	if (!max9867->master) {
 		/*
 		 * digital pll locks on to any externally supplied LRCLK signal

@@ -283,13 +227,13 @@ static int max9867_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 	/* Set the prescaler based on the master clock frequency*/
 	if (freq >= 10000000 && freq <= 20000000) {
 		value |= MAX9867_PSCLK_10_20;
-		max9867->pclk =  freq;
+		max9867->pclk = freq;
 	} else if (freq >= 20000000 && freq <= 40000000) {
 		value |= MAX9867_PSCLK_20_40;
-		max9867->pclk =  freq/2;
+		max9867->pclk = freq / 2;
 	} else if (freq >= 40000000 && freq <= 60000000) {
 		value |= MAX9867_PSCLK_40_60;
-		max9867->pclk =  freq/4;
+		max9867->pclk = freq / 4;
 	} else {
 		dev_err(component->dev,
 			"Invalid clock frequency %uHz (required 10-60MHz)\n",

@@ -366,10 +310,6 @@ static const struct snd_soc_dai_ops max9867_dai_ops = {
 	.hw_params = max9867_dai_hw_params,
 };
 
-#define MAX9867_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |\
-	SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000)
-#define MAX9867_FORMATS (SNDRV_PCM_FMTBIT_S16_LE)
-
 static struct snd_soc_dai_driver max9867_dai[] = {
 	{
 	.name = "max9867-aif1",

@@ -377,15 +317,19 @@ static struct snd_soc_dai_driver max9867_dai[] = {
 		.stream_name = "HiFi Playback",
 		.channels_min = 2,
 		.channels_max = 2,
-		.rates = MAX9867_RATES,
-		.formats = MAX9867_FORMATS,
+		.rates = SNDRV_PCM_RATE_CONTINUOUS,
+		.rate_min = 8000,
+		.rate_max = 48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
 	},
 	.capture = {
 		.stream_name = "HiFi Capture",
 		.channels_min = 2,
 		.channels_max = 2,
-		.rates = MAX9867_RATES,
-		.formats = MAX9867_FORMATS,
+		.rates = SNDRV_PCM_RATE_CONTINUOUS,
+		.rate_min = 8000,
+		.rate_max = 48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
 	},
 	.ops = &max9867_dai_ops,
 	.symmetric_rates = 1,
Signature · 1 line
2.16.2
maintainer
#16 lore raw

Re: [PATCH 6/6] ASoC: max9867: Use continuous sample rate

Quoted 5 lines · Ladislav Michl

what is exact meaning of SNDRV_PCM_RATE_CONTINUOUS? What if codec is able to set "any" rate, but there are rounding errors? Shall we pick exact matches based on master clock frequency? Few other drivers are also setting SNDRV_PCM_RATE_CONTINUOUS, but certainly cannot set any rate exactly.

It means being able to set any rate exactly. Many devices have hardware which is only specified to work at specific rates but some are more flexible. Devices don't need to be able to do this independently, you can have a flexible external clock tree for example.

author
#17 lore raw

Re: [PATCH 6/6] ASoC: max9867: Use continuous sample rate

Quoted 8 lines · Mark Brown
Quoted 5 lines · Ladislav Michl

what is exact meaning of SNDRV_PCM_RATE_CONTINUOUS? What if codec is able to set "any" rate, but there are rounding errors? Shall we pick exact matches based on master clock frequency? Few other drivers are also setting SNDRV_PCM_RATE_CONTINUOUS, but certainly cannot set any rate exactly.

Quoted 4 lines

It means being able to set any rate exactly. Many devices have hardware which is only specified to work at specific rates but some are more flexible. Devices don't need to be able to do this independently, you can have a flexible external clock tree for example.

Thanks for clarifying this. So in this particular case we should use snd_pcm_hw_constraint_list for SNDRV_PCM_HW_PARAM_RATE to enforce only supported rates based on master clock, right? I'll rework this patch then.

ladis

maintainer
#18 lore raw

Re: [PATCH 6/6] ASoC: max9867: Use continuous sample rate

Quoted 8 lines · Ladislav Michl
Quoted 2 lines · Mark Brown

flexible. Devices don't need to be able to do this independently, you can have a flexible external clock tree for example.

Thanks for clarifying this. So in this particular case we should use snd_pcm_hw_constraint_list for SNDRV_PCM_HW_PARAM_RATE to enforce only supported rates based on master clock, right? I'll rework this patch then.

Yes, that's the ideal.