[PATCH 1/3] ASoC: cs4265: SOC_SINGLE register value error fix

[PATCH 1/3] ASoC: cs4265: SOC_SINGLE register value error fix

[Matt Flax]

The cs4265 driver declares the "MMTLR Data Switch" register setting with
a 0 register value rather then the 0x12 register (CS4265_SPDIF_CTL2).
This incorrect value causes alsamixer to fault with the output :
cannot load mixer controls: Input/output error

This patch corrects the register value. alsamixer now runs.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/codecs/cs4265.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/codecs/cs4265.c b/sound/soc/codecs/cs4265.c
index 275677de..15b4ae04 100644
--- a/sound/soc/codecs/cs4265.c
+++ b/sound/soc/codecs/cs4265.c
@@ -157,8 +157,7 @@ static const struct snd_kcontrol_new cs4265_snd_controls[] = {
 	SOC_SINGLE("Validity Bit Control Switch", CS4265_SPDIF_CTL2,
 				3, 1, 0),
 	SOC_ENUM("SPDIF Mono/Stereo", spdif_mono_stereo_enum),
-	SOC_SINGLE("MMTLR Data Switch", 0,
-				1, 1, 0),
+	SOC_SINGLE("MMTLR Data Switch", CS4265_SPDIF_CTL2, 0, 1, 0),
 	SOC_ENUM("Mono Channel Select", spdif_mono_select_enum),
 	SND_SOC_BYTES("C Data Buffer", CS4265_C_DATA_BUFF, 24),
 };
-- 
2.17.1

[PATCH 2/3] ASoC: cs4265: Add native 32bit I2S transport

[Matt Flax]

The cs4265 uses 32 bit transport on the I2S bus. This patch enables native
32 bit mode for machine drivers which use this sound card driver.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/codecs/cs4265.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/cs4265.c b/sound/soc/codecs/cs4265.c
index 15b4ae04..17d7e6f0 100644
--- a/sound/soc/codecs/cs4265.c
+++ b/sound/soc/codecs/cs4265.c
@@ -495,7 +495,8 @@ static int cs4265_set_bias_level(struct snd_soc_component *component,
 			SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000)
 
 #define CS4265_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_U16_LE | \
-			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_U24_LE)
+			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_U24_LE | \
+			SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_U32_LE)
 
 static const struct snd_soc_dai_ops cs4265_ops = {
 	.hw_params	= cs4265_pcm_hw_params,
-- 
2.17.1

Re: [PATCH 2/3] ASoC: cs4265: Add native 32bit I2S transport

[Charles Keepax]

On Mon, Aug 27, 2018 at 08:58:43AM +1000, Matt Flax wrote:
> The cs4265 uses 32 bit transport on the I2S bus. This patch enables native
> 32 bit mode for machine drivers which use this sound card driver.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
>  sound/soc/codecs/cs4265.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/cs4265.c b/sound/soc/codecs/cs4265.c
> index 15b4ae04..17d7e6f0 100644
> --- a/sound/soc/codecs/cs4265.c
> +++ b/sound/soc/codecs/cs4265.c
> @@ -495,7 +495,8 @@ static int cs4265_set_bias_level(struct snd_soc_component *component,
>  			SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000)
>  
>  #define CS4265_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_U16_LE | \
> -			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_U24_LE)
> +			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_U24_LE | \
> +			SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_U32_LE)

Are you sure this is correct? The datasheet for the part only
says it supports up to 24-bit audio and I thought the defines
SNDRV_PCM_FMTBIT_S24_LE and U24 are expected to have 32-bits
in the slot whilst on the bus.

>  
>  static const struct snd_soc_dai_ops cs4265_ops = {
>  	.hw_params	= cs4265_pcm_hw_params,
> -- 
> 2.17.1

Thanks,
Charles

Re: [PATCH 2/3] ASoC: cs4265: Add native 32bit I2S transport

[Matt Flax]

On 27/08/18 18:28, Charles Keepax wrote:
> On Mon, Aug 27, 2018 at 08:58:43AM +1000, Matt Flax wrote:
>> The cs4265 uses 32 bit transport on the I2S bus. This patch enables native
>> 32 bit mode for machine drivers which use this sound card driver.
>>
>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>> ---
>>   sound/soc/codecs/cs4265.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/codecs/cs4265.c b/sound/soc/codecs/cs4265.c
>> index 15b4ae04..17d7e6f0 100644
>> --- a/sound/soc/codecs/cs4265.c
>> +++ b/sound/soc/codecs/cs4265.c
>> @@ -495,7 +495,8 @@ static int cs4265_set_bias_level(struct snd_soc_component *component,
>>   			SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000)
>>   
>>   #define CS4265_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_U16_LE | \
>> -			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_U24_LE)
>> +			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_U24_LE | \
>> +			SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_U32_LE)
> Are you sure this is correct? The datasheet for the part only
> says it supports up to 24-bit audio and I thought the defines
> SNDRV_PCM_FMTBIT_S24_LE and U24 are expected to have 32-bits
> in the slot whilst on the bus.

I will check this to make sure - leave it with me.


>>   
>>   static const struct snd_soc_dai_ops cs4265_ops = {
>>   	.hw_params	= cs4265_pcm_hw_params,
>> -- 
>> 2.17.1
> Thanks,
> Charles
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/3] ASoC: cs4265: Add native 32bit I2S transport

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 371 bytes --]

On Mon, Aug 27, 2018 at 09:28:43AM +0100, Charles Keepax wrote:

> Are you sure this is correct? The datasheet for the part only
> says it supports up to 24-bit audio and I thought the defines
> SNDRV_PCM_FMTBIT_S24_LE and U24 are expected to have 32-bits
> in the slot whilst on the bus.

No, they're not.  They're expected to be 32 bit in memory and 24 bit on
the bus.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/3] ASoC: cs4265: Add native 32bit I2S transport

[Matt Flax]

On 29/08/18 04:56, Mark Brown wrote:
> On Mon, Aug 27, 2018 at 09:28:43AM +0100, Charles Keepax wrote:
>
>> Are you sure this is correct? The datasheet for the part only
>> says it supports up to 24-bit audio and I thought the defines
>> SNDRV_PCM_FMTBIT_S24_LE and U24 are expected to have 32-bits
>> in the slot whilst on the bus.
> No, they're not.  They're expected to be 32 bit in memory and 24 bit on
> the bus.
>

If this is the case, then I would assume that 32bit formats are forced 
through ALSA plughw.

If operation is forced through plughw, then this patch is necessary to 
allow native 32 operation on the bus. The codec drops the 8 LSBs during 
operation as it reads/writes the 24 MSBs.

Matt

Re: [PATCH 2/3] ASoC: cs4265: Add native 32bit I2S transport

[Matt Flax]

On 29/08/18 09:31, Matt Flax wrote:
>
>
> On 29/08/18 04:56, Mark Brown wrote:
>> On Mon, Aug 27, 2018 at 09:28:43AM +0100, Charles Keepax wrote:
>>
>>> Are you sure this is correct? The datasheet for the part only
>>> says it supports up to 24-bit audio and I thought the defines
>>> SNDRV_PCM_FMTBIT_S24_LE and U24 are expected to have 32-bits
>>> in the slot whilst on the bus.
>> No, they're not.  They're expected to be 32 bit in memory and 24 bit on
>> the bus.
>>
>
> If this is the case, then I would assume that 32bit formats are forced 
> through ALSA plughw.
>
> If operation is forced through plughw, then this patch is necessary to 
> allow native 32 operation on the bus. The codec drops the 8 LSBs 
> during operation as it reads/writes the 24 MSBs.
>

Just to confirm this patch is necessary to support 32 bit audio formats 
naively with the cs4265 codec. I did the following tests.

Without the codec patch, limiting to 24 and 16 bit formats :
$ aplay -v tone.48k.1s.2ch.32bit.wav
aplay: set_params:1233: Sample format non available
Available formats:
- S16_LE
- S24_LE

With the codec patch, 32 bits are allowed on the I2S bus it plays as 
expected :
$ aplay -v tone.48k.1s.2ch.32bit.wav
Playing WAVE 'tone.48k.1s.2ch.32bit.wav' : Signed 32 bit Little Endian, 
Rate 48000 Hz, Stereo

Re: [PATCH 2/3] ASoC: cs4265: Add native 32bit I2S transport

[Charles Keepax]

On Wed, Aug 29, 2018 at 09:31:31AM +1000, Matt Flax wrote:
> 
> 
> On 29/08/18 04:56, Mark Brown wrote:
> >On Mon, Aug 27, 2018 at 09:28:43AM +0100, Charles Keepax wrote:
> >
> >>Are you sure this is correct? The datasheet for the part only
> >>says it supports up to 24-bit audio and I thought the defines
> >>SNDRV_PCM_FMTBIT_S24_LE and U24 are expected to have 32-bits
> >>in the slot whilst on the bus.
> >No, they're not.  They're expected to be 32 bit in memory and 24 bit on
> >the bus.
> >
> 
> If this is the case, then I would assume that 32bit formats are
> forced through ALSA plughw.
> 
> If operation is forced through plughw, then this patch is necessary
> to allow native 32 operation on the bus. The codec drops the 8 LSBs
> during operation as it reads/writes the 24 MSBs.
> 

Yeah apologies for the confusion I imagine this patch is then
fine.

Thanks,
Charles

[PATCH 3/3] ASoC: cs4265: Add a SPDIF enable actl

[Matt Flax]

This patch adds a SPDIF enable/disable toggle switch to the
sound controls.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/codecs/cs4265.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/soc/codecs/cs4265.c b/sound/soc/codecs/cs4265.c
index 17d7e6f0..cdfcca9c 100644
--- a/sound/soc/codecs/cs4265.c
+++ b/sound/soc/codecs/cs4265.c
@@ -112,6 +112,13 @@ static const char * const cam_mono_stereo_text[] = {
 static SOC_ENUM_SINGLE_DECL(spdif_mono_stereo_enum, CS4265_SPDIF_CTL2, 2,
 		cam_mono_stereo_text);
 
+static const char * const spdif_enable_text[] = {
+	"Enabled", "Disabled"
+};
+
+static SOC_ENUM_SINGLE_DECL(spdif_enable_enum, CS4265_SPDIF_CTL2, 5,
+		spdif_enable_text);
+
 static const char * const mono_select_text[] = {
 	"Channel A", "Channel B"
 };
@@ -151,6 +158,7 @@ static const struct snd_kcontrol_new cs4265_snd_controls[] = {
 				1, 1),
 	SOC_SINGLE("ADC Soft Ramp Switch", CS4265_ADC_CTL2, 7,
 				1, 0),
+	SOC_ENUM("SPDIF Enable", spdif_enable_enum),
 	SOC_SINGLE("E to F Buffer Disable Switch", CS4265_SPDIF_CTL1,
 				6, 1, 0),
 	SOC_ENUM("C Data Access", cam_mode_enum),
-- 
2.17.1

Re: [PATCH 3/3] ASoC: cs4265: Add a SPDIF enable actl

[Charles Keepax]

On Mon, Aug 27, 2018 at 08:58:44AM +1000, Matt Flax wrote:
> This patch adds a SPDIF enable/disable toggle switch to the
> sound controls.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
>  sound/soc/codecs/cs4265.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/sound/soc/codecs/cs4265.c b/sound/soc/codecs/cs4265.c
> index 17d7e6f0..cdfcca9c 100644
> --- a/sound/soc/codecs/cs4265.c
> +++ b/sound/soc/codecs/cs4265.c
> @@ -112,6 +112,13 @@ static const char * const cam_mono_stereo_text[] = {
>  static SOC_ENUM_SINGLE_DECL(spdif_mono_stereo_enum, CS4265_SPDIF_CTL2, 2,
>  		cam_mono_stereo_text);
>  
> +static const char * const spdif_enable_text[] = {
> +	"Enabled", "Disabled"
> +};

If the values are just on/off would not a SOC_SINGLE be more
appropriate?

> +
> +static SOC_ENUM_SINGLE_DECL(spdif_enable_enum, CS4265_SPDIF_CTL2, 5,
> +		spdif_enable_text);
> +
>  static const char * const mono_select_text[] = {
>  	"Channel A", "Channel B"
>  };
> @@ -151,6 +158,7 @@ static const struct snd_kcontrol_new cs4265_snd_controls[] = {
>  				1, 1),
>  	SOC_SINGLE("ADC Soft Ramp Switch", CS4265_ADC_CTL2, 7,
>  				1, 0),
> +	SOC_ENUM("SPDIF Enable", spdif_enable_enum),

Although you would want to then call this something like "SPDIF
Switch".

>  	SOC_SINGLE("E to F Buffer Disable Switch", CS4265_SPDIF_CTL1,
>  				6, 1, 0),
>  	SOC_ENUM("C Data Access", cam_mode_enum),
> -- 
> 2.17.1

Thanks,
Charles

Re: [PATCH 3/3] ASoC: cs4265: Add a SPDIF enable actl

[Matt Flax]

On 27/08/18 18:24, Charles Keepax wrote:
> On Mon, Aug 27, 2018 at 08:58:44AM +1000, Matt Flax wrote:
>> This patch adds a SPDIF enable/disable toggle switch to the
>> sound controls.
>>
>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>> ---
>>   sound/soc/codecs/cs4265.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/sound/soc/codecs/cs4265.c b/sound/soc/codecs/cs4265.c
>> index 17d7e6f0..cdfcca9c 100644
>> --- a/sound/soc/codecs/cs4265.c
>> +++ b/sound/soc/codecs/cs4265.c
>> @@ -112,6 +112,13 @@ static const char * const cam_mono_stereo_text[] = {
>>   static SOC_ENUM_SINGLE_DECL(spdif_mono_stereo_enum, CS4265_SPDIF_CTL2, 2,
>>   		cam_mono_stereo_text);
>>   
>> +static const char * const spdif_enable_text[] = {
>> +	"Enabled", "Disabled"
>> +};
> If the values are just on/off would not a SOC_SINGLE be more
> appropriate?

OK that sounds like a better idea, I will implement a SOC_SINGLE. Thanks 
for the guidance.

>
>> +
>> +static SOC_ENUM_SINGLE_DECL(spdif_enable_enum, CS4265_SPDIF_CTL2, 5,
>> +		spdif_enable_text);
>> +
>>   static const char * const mono_select_text[] = {
>>   	"Channel A", "Channel B"
>>   };
>> @@ -151,6 +158,7 @@ static const struct snd_kcontrol_new cs4265_snd_controls[] = {
>>   				1, 1),
>>   	SOC_SINGLE("ADC Soft Ramp Switch", CS4265_ADC_CTL2, 7,
>>   				1, 0),
>> +	SOC_ENUM("SPDIF Enable", spdif_enable_enum),
> Although you would want to then call this something like "SPDIF
> Switch".

Yes, sounds good.

>
>>   	SOC_SINGLE("E to F Buffer Disable Switch", CS4265_SPDIF_CTL1,
>>   				6, 1, 0),
>>   	SOC_ENUM("C Data Access", cam_mode_enum),
>> -- 
>> 2.17.1
> Thanks,
> Charles
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ASoC: cs4265: SOC_SINGLE register value error fix

[Charles Keepax]

On Mon, Aug 27, 2018 at 08:58:42AM +1000, Matt Flax wrote:
> The cs4265 driver declares the "MMTLR Data Switch" register setting with
> a 0 register value rather then the 0x12 register (CS4265_SPDIF_CTL2).
> This incorrect value causes alsamixer to fault with the output :
> cannot load mixer controls: Input/output error
> 
> This patch corrects the register value. alsamixer now runs.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH 1/3] ASoC: cs4265: SOC_SINGLE register value error fix

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 437 bytes --]

On Mon, Aug 27, 2018 at 08:58:42AM +1000, Matt Flax wrote:
> The cs4265 driver declares the "MMTLR Data Switch" register setting with
> a 0 register value rather then the 0x12 register (CS4265_SPDIF_CTL2).
> This incorrect value causes alsamixer to fault with the output :
> cannot load mixer controls: Input/output error

How is this patch series different to the apparently identical three
patch series you sent a few minutes earlier?

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/3] ASoC: cs4265: SOC_SINGLE register value error fix

[Matt Flax]

On 29/08/18 04:57, Mark Brown wrote:
> On Mon, Aug 27, 2018 at 08:58:42AM +1000, Matt Flax wrote:
>> The cs4265 driver declares the "MMTLR Data Switch" register setting with
>> a 0 register value rather then the 0x12 register (CS4265_SPDIF_CTL2).
>> This incorrect value causes alsamixer to fault with the output :
>> cannot load mixer controls: Input/output error
> How is this patch series different to the apparently identical three
> patch series you sent a few minutes earlier?
>

The identical patch series were sent from the wrong email address and 
didn't go through to the list - because of moderation.
I apologise for the first mistake.

Matt

[PATCH 1/3] ASoC: cs4265: SOC_SINGLE register value error fix

[Matt Flax]

From: Matt Flax <flatmax@flatmax.org>

The cs4265 driver declares the "MMTLR Data Switch" register setting with
a 0 register value rather then the 0x12 register (CS4265_SPDIF_CTL2).
This incorrect value causes alsamixer to fault with the output :
cannot load mixer controls: Input/output error

This patch corrects the register value. alsamixer now runs.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/codecs/cs4265.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/codecs/cs4265.c b/sound/soc/codecs/cs4265.c
index 275677de..15b4ae04 100644
--- a/sound/soc/codecs/cs4265.c
+++ b/sound/soc/codecs/cs4265.c
@@ -157,8 +157,7 @@ static const struct snd_kcontrol_new cs4265_snd_controls[] = {
 	SOC_SINGLE("Validity Bit Control Switch", CS4265_SPDIF_CTL2,
 				3, 1, 0),
 	SOC_ENUM("SPDIF Mono/Stereo", spdif_mono_stereo_enum),
-	SOC_SINGLE("MMTLR Data Switch", 0,
-				1, 1, 0),
+	SOC_SINGLE("MMTLR Data Switch", CS4265_SPDIF_CTL2, 0, 1, 0),
 	SOC_ENUM("Mono Channel Select", spdif_mono_select_enum),
 	SND_SOC_BYTES("C Data Buffer", CS4265_C_DATA_BUFF, 24),
 };
-- 
2.17.1