[RFC,10/34] ASoC: sun8i-codec: Advertise only hardware-supported rates
diff mbox series

Message ID 20200217064250.15516-11-samuel@sholland.org
State New
Headers show
Series
  • sun8i-codec fixes and new features
Related show

Commit Message

Samuel Holland Feb. 17, 2020, 6:42 a.m. UTC
The hardware does not support 64kHz, 88.2kHz, or 176.4kHz sample rates,
so the driver should not advertise them. The hardware can handle two
additional non-standard sample rates: 12kHz and 24kHz, so declare
support for them via SNDRV_PCM_RATE_KNOT.

Cc: stable@kernel.org
Fixes: 36c684936fae ("ASoC: Add sun8i digital audio codec")
Fixes: eda85d1fee05 ("ASoC: sun8i-codec: Add ADC support for a33")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 sound/soc/sunxi/sun8i-codec.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Mark Brown Feb. 17, 2020, 3:30 p.m. UTC | #1
On Mon, Feb 17, 2020 at 12:42:26AM -0600, Samuel Holland wrote:
> The hardware does not support 64kHz, 88.2kHz, or 176.4kHz sample rates,
> so the driver should not advertise them. The hardware can handle two
> additional non-standard sample rates: 12kHz and 24kHz, so declare
> support for them via SNDRV_PCM_RATE_KNOT.
> 
> Cc: stable@kernel.org
> Fixes: 36c684936fae ("ASoC: Add sun8i digital audio codec")
> Fixes: eda85d1fee05 ("ASoC: sun8i-codec: Add ADC support for a33")
> Signed-off-by: Samuel Holland <samuel@sholland.org>

The new sample rates are new functionality, they are definitely not
stable material.   For the sample rates you are removing do we
understand why they were added - do they work for people, are they
perhaps supported for some users and not others for example?
Samuel Holland Feb. 18, 2020, 1:55 a.m. UTC | #2
On 2/17/20 9:30 AM, Mark Brown wrote:
> On Mon, Feb 17, 2020 at 12:42:26AM -0600, Samuel Holland wrote:
>> The hardware does not support 64kHz, 88.2kHz, or 176.4kHz sample rates,
>> so the driver should not advertise them. The hardware can handle two
>> additional non-standard sample rates: 12kHz and 24kHz, so declare
>> support for them via SNDRV_PCM_RATE_KNOT.
>>
>> Cc: stable@kernel.org
>> Fixes: 36c684936fae ("ASoC: Add sun8i digital audio codec")
>> Fixes: eda85d1fee05 ("ASoC: sun8i-codec: Add ADC support for a33")
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
> 
> The new sample rates are new functionality, they are definitely not
> stable material.   For the sample rates you are removing do we
> understand why they were added - do they work for people, are they
> perhaps supported for some users and not others for example?

I do not know why they were added, but the sample rates I removed do not work
today, for anyone.

The sample rate must be programmed into the hardware, and the removed sample
rates do not map to one of the possible register values, so
sun8i_codec_get_hw_rate(), and thus hw_params, will return -EINVAL if one of
them is used.

Patch
diff mbox series

diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
index dca6f4b9d4b8..bf12f5199e96 100644
--- a/sound/soc/sunxi/sun8i-codec.c
+++ b/sound/soc/sunxi/sun8i-codec.c
@@ -86,6 +86,11 @@ 
 #define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV_MASK	GENMASK(8, 6)
 #define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK	GENMASK(12, 9)
 
+#define SUN8I_AIF_PCM_RATES (SNDRV_PCM_RATE_8000_48000|\
+			     SNDRV_PCM_RATE_96000|\
+			     SNDRV_PCM_RATE_192000|\
+			     SNDRV_PCM_RATE_KNOT)
+
 struct sun8i_codec {
 	struct regmap	*regmap;
 	struct clk	*clk_module;
@@ -515,7 +520,7 @@  static struct snd_soc_dai_driver sun8i_codec_dai = {
 		.stream_name = "Playback",
 		.channels_min = 1,
 		.channels_max = 2,
-		.rates = SNDRV_PCM_RATE_8000_192000,
+		.rates = SUN8I_AIF_PCM_RATES,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE,
 	},
 	/* capture capabilities */
@@ -523,7 +528,7 @@  static struct snd_soc_dai_driver sun8i_codec_dai = {
 		.stream_name = "Capture",
 		.channels_min = 1,
 		.channels_max = 2,
-		.rates = SNDRV_PCM_RATE_8000_192000,
+		.rates = SUN8I_AIF_PCM_RATES,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE,
 		.sig_bits = 24,
 	},