diff mbox

ASoC: arizona: Add support for SNDRV_PCM_RATE_KNOT

Message ID 1454431084-21411-1-git-send-email-ckeepax@opensource.wolfsonmicro.com (mailing list archive)
State New, archived
Headers show

Commit Message

Charles Keepax Feb. 2, 2016, 4:38 p.m. UTC 
The Arizona CODECs support several rates that do not have simple defines
in ALSA. This patch adds support for SNDRV_PCM_RATE_KNOT so that users
can open stream at these rates. As part of this we should always set
constraints in arizona_startup, currently we only set the constraints if
we already have a clock to limit rates to that family of sample rates.
This patch updates this to set a constraint of all rates supported by
the chip if we do not already know which family of rates to limit to.
Finally we also reduce the list of rates supported in the constraints to
only include those that are supported on current parts.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/arizona.c | 47 ++++++++++++++++++----------------------------
 sound/soc/codecs/cs47l24.c |  2 +-
 sound/soc/codecs/wm5102.c  |  2 +-
 sound/soc/codecs/wm5110.c  |  2 +-
 sound/soc/codecs/wm8997.c  |  2 +-
 sound/soc/codecs/wm8998.c  |  2 +-
 6 files changed, 23 insertions(+), 34 deletions(-)

Comments

Mark Brown Feb. 2, 2016, 7:30 p.m. UTC | #1 
On Tue, Feb 02, 2016 at 04:38:04PM +0000, Charles Keepax wrote:

> +#define ARIZONA_48K_RATES 12000, 24000, 48000, 96000, 192000, 4000, 8000, \
> +			  16000, 32000

This just looks wrong.  It's valid code but with the combination of the
split line and the list without any sort of parenthesis it sets off
alarm bells.  Duplicating might not be the worst thing ever...
Lars-Peter Clausen Feb. 3, 2016, 8:24 a.m. UTC | #2 
On 02/02/2016 08:30 PM, Mark Brown wrote:
> On Tue, Feb 02, 2016 at 04:38:04PM +0000, Charles Keepax wrote:
> 
>> +#define ARIZONA_48K_RATES 12000, 24000, 48000, 96000, 192000, 4000, 8000, \
>> +			  16000, 32000
> 
> This just looks wrong.  It's valid code but with the combination of the
> split line and the list without any sort of parenthesis it sets off
> alarm bells.  Duplicating might not be the worst thing ever...

For list constraints it is possible to specify a mask for which of the
entries in the rates array should be considered. So you could use the same
rate array, but use different masks. The adau1977 drive for example does this.
Charles Keepax Feb. 3, 2016, 9:41 a.m. UTC | #3 
On Wed, Feb 03, 2016 at 09:24:25AM +0100, Lars-Peter Clausen wrote:
> On 02/02/2016 08:30 PM, Mark Brown wrote:
> > On Tue, Feb 02, 2016 at 04:38:04PM +0000, Charles Keepax wrote:
> > 
> >> +#define ARIZONA_48K_RATES 12000, 24000, 48000, 96000, 192000, 4000, 8000, \
> >> +			  16000, 32000
> > 
> > This just looks wrong.  It's valid code but with the combination of the
> > split line and the list without any sort of parenthesis it sets off
> > alarm bells.  Duplicating might not be the worst thing ever...
> 
> For list constraints it is possible to specify a mask for which of the
> entries in the rates array should be considered. So you could use the same
> rate array, but use different masks. The adau1977 drive for example does this.
> 
> 

Thanks wasn't aware of that I will have a look and respin the
patch.

Thanks,
Charles
Charles Keepax Feb. 4, 2016, 12:45 p.m. UTC | #4 
On Wed, Feb 03, 2016 at 09:24:25AM +0100, Lars-Peter Clausen wrote:
> On 02/02/2016 08:30 PM, Mark Brown wrote:
> > On Tue, Feb 02, 2016 at 04:38:04PM +0000, Charles Keepax wrote:
> > 
> >> +#define ARIZONA_48K_RATES 12000, 24000, 48000, 96000, 192000, 4000, 8000, \
> >> +			  16000, 32000
> > 
> > This just looks wrong.  It's valid code but with the combination of the
> > split line and the list without any sort of parenthesis it sets off
> > alarm bells.  Duplicating might not be the worst thing ever...
> 
> For list constraints it is possible to specify a mask for which of the
> entries in the rates array should be considered. So you could use the same
> rate array, but use different masks. The adau1977 drive for example does this.
> 

Ok so looking at this a little more I think you can only apply
the mask thing to values that are part of the basic defines
rather than masking off entries in a constraint list. The
adau1977 is using the mask to limit the supported formats which
is all done through the defines in that driver.

Thanks,
Charles
Charles Keepax Feb. 4, 2016, 12:46 p.m. UTC | #5 
On Tue, Feb 02, 2016 at 07:30:02PM +0000, Mark Brown wrote:
> On Tue, Feb 02, 2016 at 04:38:04PM +0000, Charles Keepax wrote:
> 
> > +#define ARIZONA_48K_RATES 12000, 24000, 48000, 96000, 192000, 4000, 8000, \
> > +			  16000, 32000
> 
> This just looks wrong.  It's valid code but with the combination of the
> split line and the list without any sort of parenthesis it sets off
> alarm bells.  Duplicating might not be the worst thing ever...

I will respin with just duplicating the rates lists, I am happy
enough with that.

Thanks,
Charles
Lars-Peter Clausen Feb. 4, 2016, 12:55 p.m. UTC | #6 
On 02/04/2016 01:45 PM, Charles Keepax wrote:
> On Wed, Feb 03, 2016 at 09:24:25AM +0100, Lars-Peter Clausen wrote:
>> On 02/02/2016 08:30 PM, Mark Brown wrote:
>>> On Tue, Feb 02, 2016 at 04:38:04PM +0000, Charles Keepax wrote:
>>>
>>>> +#define ARIZONA_48K_RATES 12000, 24000, 48000, 96000, 192000, 4000, 8000, \
>>>> +			  16000, 32000
>>>
>>> This just looks wrong.  It's valid code but with the combination of the
>>> split line and the list without any sort of parenthesis it sets off
>>> alarm bells.  Duplicating might not be the worst thing ever...
>>
>> For list constraints it is possible to specify a mask for which of the
>> entries in the rates array should be considered. So you could use the same
>> rate array, but use different masks. The adau1977 drive for example does this.
>>
> 
> Ok so looking at this a little more I think you can only apply
> the mask thing to values that are part of the basic defines
> rather than masking off entries in a constraint list. The
> adau1977 is using the mask to limit the supported formats which
> is all done through the defines in that driver.

The list constraint struct looks like this:

struct snd_pcm_hw_constraint_list {
        unsigned int count;
        const unsigned int *list;
        unsigned int mask;
};

list is your rates array, count is the size of the rates array and mask
specifies which of the entries in the rates array are valid.

E.g.

unsigned int rates[] = {
	44100,
	48000,
};

struct snd_pcm_hw_constraint_list constr_44100 {
	.count = ARRAY_SIZE(rates),
	.list = rates,
	.mask = 0x1,
};

struct snd_pcm_hw_constraint_list constr_48000 {
	.count = ARRAY_SIZE(rates),
	.list = rates,
	.mask = 0x2,
};

struct snd_pcm_hw_constraint_list constr_all {
	.count = ARRAY_SIZE(rates),
	.list = rates,
	.mask = 0, /* 0 = All */
};
Charles Keepax Feb. 4, 2016, 2:34 p.m. UTC | #7 
On Thu, Feb 04, 2016 at 01:55:58PM +0100, Lars-Peter Clausen wrote:
> On 02/04/2016 01:45 PM, Charles Keepax wrote:
> > On Wed, Feb 03, 2016 at 09:24:25AM +0100, Lars-Peter Clausen wrote:
> >> On 02/02/2016 08:30 PM, Mark Brown wrote:
> >>> On Tue, Feb 02, 2016 at 04:38:04PM +0000, Charles Keepax wrote:
> >>>
> >>>> +#define ARIZONA_48K_RATES 12000, 24000, 48000, 96000, 192000, 4000, 8000, \
> >>>> +			  16000, 32000
> >>>
> >>> This just looks wrong.  It's valid code but with the combination of the
> >>> split line and the list without any sort of parenthesis it sets off
> >>> alarm bells.  Duplicating might not be the worst thing ever...
> >>
> >> For list constraints it is possible to specify a mask for which of the
> >> entries in the rates array should be considered. So you could use the same
> >> rate array, but use different masks. The adau1977 drive for example does this.
> >>
> > 
> > Ok so looking at this a little more I think you can only apply
> > the mask thing to values that are part of the basic defines
> > rather than masking off entries in a constraint list. The
> > adau1977 is using the mask to limit the supported formats which
> > is all done through the defines in that driver.
> 
> The list constraint struct looks like this:
> 
> struct snd_pcm_hw_constraint_list {
>         unsigned int count;
>         const unsigned int *list;
>         unsigned int mask;
> };
> 
> list is your rates array, count is the size of the rates array and mask
> specifies which of the entries in the rates array are valid.
> 
> E.g.
> 
> unsigned int rates[] = {
> 	44100,
> 	48000,
> };
> 
> struct snd_pcm_hw_constraint_list constr_44100 {
> 	.count = ARRAY_SIZE(rates),
> 	.list = rates,
> 	.mask = 0x1,
> };
> 
> struct snd_pcm_hw_constraint_list constr_48000 {
> 	.count = ARRAY_SIZE(rates),
> 	.list = rates,
> 	.mask = 0x2,
> };
> 
> struct snd_pcm_hw_constraint_list constr_all {
> 	.count = ARRAY_SIZE(rates),
> 	.list = rates,
> 	.mask = 0, /* 0 = All */
> };
> 

Ah my bad I was looking at entirely the wrong mask.

Thanks,
Charles
diff mbox

Patch

diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
index 33143fe..5b16834 100644
--- a/sound/soc/codecs/arizona.c
+++ b/sound/soc/codecs/arizona.c
@@ -1398,23 +1398,10 @@  static const int arizona_48k_bclk_rates[] = {
 	24576000,
 };
 
-static const unsigned int arizona_48k_rates[] = {
-	12000,
-	24000,
-	48000,
-	96000,
-	192000,
-	384000,
-	768000,
-	4000,
-	8000,
-	16000,
-	32000,
-	64000,
-	128000,
-	256000,
-	512000,
-};
+#define ARIZONA_48K_RATES 12000, 24000, 48000, 96000, 192000, 4000, 8000, \
+			  16000, 32000
+
+static const unsigned int arizona_48k_rates[] = { ARIZONA_48K_RATES };
 
 static const struct snd_pcm_hw_constraint_list arizona_48k_constraint = {
 	.count	= ARRAY_SIZE(arizona_48k_rates),
@@ -1443,15 +1430,9 @@  static const int arizona_44k1_bclk_rates[] = {
 	22579200,
 };
 
-static const unsigned int arizona_44k1_rates[] = {
-	11025,
-	22050,
-	44100,
-	88200,
-	176400,
-	352800,
-	705600,
-};
+#define ARIZONA_44K1_RATES 11025, 22050, 44100, 88200, 176400
+
+static const unsigned int arizona_44k1_rates[] = { ARIZONA_44K1_RATES };
 
 static const struct snd_pcm_hw_constraint_list arizona_44k1_constraint = {
 	.count	= ARRAY_SIZE(arizona_44k1_rates),
@@ -1485,6 +1466,15 @@  static int arizona_sr_vals[] = {
 	512000,
 };
 
+static const unsigned int arizona_rates[] = {
+	ARIZONA_48K_RATES, ARIZONA_44K1_RATES
+};
+
+static const struct snd_pcm_hw_constraint_list arizona_constraint = {
+	.count	= ARRAY_SIZE(arizona_rates),
+	.list	= arizona_rates,
+};
+
 static int arizona_startup(struct snd_pcm_substream *substream,
 			   struct snd_soc_dai *dai)
 {
@@ -1509,9 +1499,8 @@  static int arizona_startup(struct snd_pcm_substream *substream,
 	}
 
 	if (base_rate == 0)
-		return 0;
-
-	if (base_rate % 8000)
+		constraint = &arizona_constraint;
+	else if (base_rate % 8000)
 		constraint = &arizona_44k1_constraint;
 	else
 		constraint = &arizona_48k_constraint;
diff --git a/sound/soc/codecs/cs47l24.c b/sound/soc/codecs/cs47l24.c
index b8d3cd0..576087b 100644
--- a/sound/soc/codecs/cs47l24.c
+++ b/sound/soc/codecs/cs47l24.c
@@ -923,7 +923,7 @@  static int cs47l24_set_fll(struct snd_soc_codec *codec, int fll_id, int source,
 	}
 }
 
-#define CS47L24_RATES SNDRV_PCM_RATE_8000_192000
+#define CS47L24_RATES SNDRV_PCM_RATE_KNOT
 
 #define CS47L24_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
 			 SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
diff --git a/sound/soc/codecs/wm5102.c b/sound/soc/codecs/wm5102.c
index aa1e96a..a8b3e3f 100644
--- a/sound/soc/codecs/wm5102.c
+++ b/sound/soc/codecs/wm5102.c
@@ -1738,7 +1738,7 @@  static int wm5102_set_fll(struct snd_soc_codec *codec, int fll_id, int source,
 	}
 }
 
-#define WM5102_RATES SNDRV_PCM_RATE_8000_192000
+#define WM5102_RATES SNDRV_PCM_RATE_KNOT
 
 #define WM5102_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
index 302a8d3..83ba70f 100644
--- a/sound/soc/codecs/wm5110.c
+++ b/sound/soc/codecs/wm5110.c
@@ -2024,7 +2024,7 @@  static int wm5110_set_fll(struct snd_soc_codec *codec, int fll_id, int source,
 	}
 }
 
-#define WM5110_RATES SNDRV_PCM_RATE_8000_192000
+#define WM5110_RATES SNDRV_PCM_RATE_KNOT
 
 #define WM5110_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
diff --git a/sound/soc/codecs/wm8997.c b/sound/soc/codecs/wm8997.c
index b4dba3a..52d766e 100644
--- a/sound/soc/codecs/wm8997.c
+++ b/sound/soc/codecs/wm8997.c
@@ -943,7 +943,7 @@  static int wm8997_set_fll(struct snd_soc_codec *codec, int fll_id, int source,
 	}
 }
 
-#define WM8997_RATES SNDRV_PCM_RATE_8000_192000
+#define WM8997_RATES SNDRV_PCM_RATE_KNOT
 
 #define WM8997_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
diff --git a/sound/soc/codecs/wm8998.c b/sound/soc/codecs/wm8998.c
index 7719bc5..0123960 100644
--- a/sound/soc/codecs/wm8998.c
+++ b/sound/soc/codecs/wm8998.c
@@ -1170,7 +1170,7 @@  static const struct snd_soc_dapm_route wm8998_dapm_routes[] = {
 	{ "DRC1 Signal Activity", NULL, "DRC1R" },
 };
 
-#define WM8998_RATES SNDRV_PCM_RATE_8000_192000
+#define WM8998_RATES SNDRV_PCM_RATE_KNOT
 
 #define WM8998_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)