[PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Qiao Zhou]

add TISSP mode support. with this mode enabled, ssp controller
works in TI-ssp protocol automatically and doesn't need to
configure sspsp register to set the timing for tx/rx.

Signed-off-by: Qiao Zhou <zhouqiao@marvell.com>
---
 include/sound/soc-dai.h |    1 +
 sound/soc/pxa/pxa-ssp.c |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index ae9a227..43fc5d0 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -33,6 +33,7 @@ struct snd_compr_stream;
 #define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
 #define SND_SOC_DAIFMT_AC97		6 /* AC97 */
 #define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */
+#define SND_SOC_DAIFMT_SSP		8 /* SSP mode */
 
 /* left and right justified also known as MSB and LSB respectively */
 #define SND_SOC_DAIFMT_MSB		SND_SOC_DAIFMT_LEFT_J
diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
index a3119a0..effa8d1 100644
--- a/sound/soc/pxa/pxa-ssp.c
+++ b/sound/soc/pxa/pxa-ssp.c
@@ -505,6 +505,14 @@ static int pxa_ssp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		sscr0 |= SSCR0_MOD | SSCR0_PSP;
 		sscr1 |= SSCR1_TRAIL | SSCR1_RWOT;
 		break;
+	/*
+	 * add TISSP protocol support. setting to this mode, ssp controller
+	 * works in TI-ssp mode, and need not set psp register to conf
+	 * detailed timing.
+	 */
+	case SND_SOC_DAIFMT_SSP:
+		sscr0 |= SSCR0_TISSP;
+		sscr1 |= SSCR1_RWOT;
 
 	default:
 		return -EINVAL;
-- 
1.7.0.4

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Mark Brown]

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

On Tue, Nov 05, 2013 at 04:30:08PM +0800, Qiao Zhou wrote:
> add TISSP mode support. with this mode enabled, ssp controller
> works in TI-ssp protocol automatically and doesn't need to
> configure sspsp register to set the timing for tx/rx.

What is TISSP mode in an audio context?

> +	/*
> +	 * add TISSP protocol support. setting to this mode, ssp controller
> +	 * works in TI-ssp mode, and need not set psp register to conf
> +	 * detailed timing.
> +	 */
> +	case SND_SOC_DAIFMT_SSP:
> +		sscr0 |= SSCR0_TISSP;
> +		sscr1 |= SSCR1_RWOT;
>  
>  	default:
>  		return -EINVAL;

I don't think this code has been tested...

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Qiao Zhou]

On 11/06/2013 07:24 PM, Mark Brown wrote:
> On Tue, Nov 05, 2013 at 04:30:08PM +0800, Qiao Zhou wrote:
>> add TISSP mode support. with this mode enabled, ssp controller
>> works in TI-ssp protocol automatically and doesn't need to
>> configure sspsp register to set the timing for tx/rx.
>
> What is TISSP mode in an audio context?
It's normal SSP mode, and it has a pulse of one bit clock width, which 
is the frame start signal. previously it's mainly used for voice call 
which is mono stream. current I2S or DSP_x format use PSP, which 
configures the details of transmit/receive timing. we can also use PSP 
to suit this SSP format. But since the SSP controller provides such HW 
convenience, so we want to add it. I'm not sure it's proper, and want to 
ask for suggestions.
>
>> +	/*
>> +	 * add TISSP protocol support. setting to this mode, ssp controller
>> +	 * works in TI-ssp mode, and need not set psp register to conf
>> +	 * detailed timing.
>> +	 */
>> +	case SND_SOC_DAIFMT_SSP:
>> +		sscr0 |= SSCR0_TISSP;
>> +		sscr1 |= SSCR1_RWOT;
>>
>>   	default:
>>   		return -EINVAL;
>
> I don't think this code has been tested...
sorry for missing the "break", will update it. I want to ask suggestions 
about the logic of this piece of code from mainline and we do test the 
code logic on our platform.
>
-- 

Best Regards
Qiao

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Mark Brown]

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

On Fri, Nov 08, 2013 at 09:52:16AM +0800, Qiao Zhou wrote:
> On 11/06/2013 07:24 PM, Mark Brown wrote:

> >What is TISSP mode in an audio context?

> It's normal SSP mode, and it has a pulse of one bit clock width,

OK, what is SSP mode?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Qiao Zhou]

On 11/08/2013 06:19 PM, Mark Brown wrote:
> On Fri, Nov 08, 2013 at 09:52:16AM +0800, Qiao Zhou wrote:
>> On 11/06/2013 07:24 PM, Mark Brown wrote:
>
>>> What is TISSP mode in an audio context?
>
>> It's normal SSP mode, and it has a pulse of one bit clock width,
>
> OK, what is SSP mode?
SSP mode is just one frame format defined in sscr0 register, and there 
are 3 other mode, such as SPI mode, National Semiconductor 
Microwire(NSM) mode, Programmable Serial Protocol(PSP) mode.
The SPI/NSM are not used for audio usage, and currently only PSP mode is 
used pxa-ssp driver. We have the requirement now to support this SSP 
mode. The benefit is that we don't need to configure detailed timing in 
transmit/receiving since HW handles it automatically.
The SSP mode is something like the PCM format for mono stream for voice 
related scenario. In current audio format definition and pxa-ssp driver 
implementation, I don't find suitable way to handle it. so I add such 
SSP-format to differentiate it from PSP mode. Do I make a confusion of 
SSP mode and audio frame format? please help comment. thanks.
>


-- 

Best Regards
Qiao

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Daniel Mack]

On 11/08/2013 02:52 AM, Qiao Zhou wrote:
> On 11/06/2013 07:24 PM, Mark Brown wrote:
>> On Tue, Nov 05, 2013 at 04:30:08PM +0800, Qiao Zhou wrote:
>>> add TISSP mode support. with this mode enabled, ssp controller
>>> works in TI-ssp protocol automatically and doesn't need to
>>> configure sspsp register to set the timing for tx/rx.
>>
>> What is TISSP mode in an audio context?
> It's normal SSP mode, and it has a pulse of one bit clock width, which 
> is the frame start signal. previously it's mainly used for voice call 
> which is mono stream. current I2S or DSP_x format use PSP, which 
> configures the details of transmit/receive timing. we can also use PSP 
> to suit this SSP format. But since the SSP controller provides such HW 
> convenience, so we want to add it. I'm not sure it's proper, and want to 
> ask for suggestions.

This sounds like a special case you want to address, rather than
something that should be recognized by the core.

The question is what you want to achive here. What kind of peripheral do
you have connected to the other side of the PXA, and why does it need
this special needle peak on the frame clock rather than a 50% duty-cycle?

Also, I vaguely remember huge problems we had in setting up the SSP
controller of a PXA SoC. The SSP IP would do all sort of things instead
of what's described in the docs. Which silicon are you working on?


Thanks,
Daniel

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Mark Brown]

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

On Sun, Nov 10, 2013 at 09:50:21AM +0800, Qiao Zhou wrote:
> On 11/08/2013 06:19 PM, Mark Brown wrote:

> The SPI/NSM are not used for audio usage, and currently only PSP
> mode is used pxa-ssp driver. We have the requirement now to support
> this SSP mode. The benefit is that we don't need to configure
> detailed timing in transmit/receiving since HW handles it
> automatically.

> The SSP mode is something like the PCM format for mono stream for
> voice related scenario. In current audio format definition and
> pxa-ssp driver implementation, I don't find suitable way to handle
> it. so I add such SSP-format to differentiate it from PSP mode. Do I
> make a confusion of SSP mode and audio frame format? please help
> comment. thanks.

If this is just about programming the hardware better rather than
changing the externally visible symbols then as Daniel says it should be
handled transparently by the driver, replacing the current code for
whatever formats are handled.  Normally the same DAI format should be
configured for both CODEC and CPU so unless a new format should also be
used by CODEC drivers the update ought to be internal to the driver.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Qiao Zhou]

On 11/10/2013 05:29 PM, Daniel Mack wrote:
> On 11/08/2013 02:52 AM, Qiao Zhou wrote:
>> On 11/06/2013 07:24 PM, Mark Brown wrote:
>>> On Tue, Nov 05, 2013 at 04:30:08PM +0800, Qiao Zhou wrote:
>>>> add TISSP mode support. with this mode enabled, ssp controller
>>>> works in TI-ssp protocol automatically and doesn't need to
>>>> configure sspsp register to set the timing for tx/rx.
>>>
>>> What is TISSP mode in an audio context?
>> It's normal SSP mode, and it has a pulse of one bit clock width, which
>> is the frame start signal. previously it's mainly used for voice call
>> which is mono stream. current I2S or DSP_x format use PSP, which
>> configures the details of transmit/receive timing. we can also use PSP
>> to suit this SSP format. But since the SSP controller provides such HW
>> convenience, so we want to add it. I'm not sure it's proper, and want to
>> ask for suggestions.
>
> This sounds like a special case you want to address, rather than
> something that should be recognized by the core.
>
> The question is what you want to achive here. What kind of peripheral do
> you have connected to the other side of the PXA, and why does it need
> this special needle peak on the frame clock rather than a 50% duty-cycle?
The frame format is like the DSP Mode A in WM8994 spec, and the 
peripheral is a normal codec interface for voice call usage, 
mono-stream, 16 bits.
Here I want to add such SSP mode support in pxa-ssp driver, instead of 
using PSP mode to configure detailed data format in PSP register.
should I handle it under DSP_A case with extra param notifying whether 
it's SSP mode or PSP mode? on the peripheral side, the configuration of 
CODEC DAI should be the same.

below are the modes definition in pxa2xx_ssp.h
#define SSCR0_TISSP	(1 << 4)	/* TI Sync Serial Protocol */
#define SSCR0_PSP	(3 << 4)	/* PSP - Programmable Serial Protocol */
>
> Also, I vaguely remember huge problems we had in setting up the SSP
> controller of a PXA SoC. The SSP IP would do all sort of things instead
> of what's described in the docs. Which silicon are you working on?
The silicon now I'm working on is pxa-9xx chips, but it's the same back 
to pxa-3xx chips.
>
>
> Thanks,
> Daniel
>
-- 

Best Regards
Qiao

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Qiao Zhou]

On 11/10/2013 07:10 PM, Mark Brown wrote:
> On Sun, Nov 10, 2013 at 09:50:21AM +0800, Qiao Zhou wrote:
>> On 11/08/2013 06:19 PM, Mark Brown wrote:
>
>> The SPI/NSM are not used for audio usage, and currently only PSP
>> mode is used pxa-ssp driver. We have the requirement now to support
>> this SSP mode. The benefit is that we don't need to configure
>> detailed timing in transmit/receiving since HW handles it
>> automatically.
>
>> The SSP mode is something like the PCM format for mono stream for
>> voice related scenario. In current audio format definition and
>> pxa-ssp driver implementation, I don't find suitable way to handle
>> it. so I add such SSP-format to differentiate it from PSP mode. Do I
>> make a confusion of SSP mode and audio frame format? please help
>> comment. thanks.
>
> If this is just about programming the hardware better rather than
> changing the externally visible symbols then as Daniel says it should be
> handled transparently by the driver, replacing the current code for
> whatever formats are handled.  Normally the same DAI format should be
> configured for both CODEC and CPU so unless a new format should also be
> used by CODEC drivers the update ought to be internal to the driver.
>
Yes, it should be an improvement of current driver. The format is not a 
new one, and the DSP_A mode is suitable for it. The question here is to 
handle both PSP and SSP mode here. should I pass another parameter here 
to support it?

-- 

Best Regards
Qiao

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Mark Brown]

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

On Mon, Nov 11, 2013 at 10:03:46AM +0800, Qiao Zhou wrote:

> Yes, it should be an improvement of current driver. The format is
> not a new one, and the DSP_A mode is suitable for it. The question
> here is to handle both PSP and SSP mode here. should I pass another
> parameter here to support it?

Can't the driver just do whatever is best automatically - why would the
user need to configure things?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [PATCH V0] ASoC: pxa-ssp: add TISSP mode support

[Qiao Zhou]

On 11/11/2013 07:21 PM, Mark Brown wrote:
> On Mon, Nov 11, 2013 at 10:03:46AM +0800, Qiao Zhou wrote:
>
>> Yes, it should be an improvement of current driver. The format is
>> not a new one, and the DSP_A mode is suitable for it. The question
>> here is to handle both PSP and SSP mode here. should I pass another
>> parameter here to support it?
>
> Can't the driver just do whatever is best automatically - why would the
> user need to configure things?
>
Hi Mark,

I'll check and replace the PSP mode setting with SSP for DSP_x mode. 
Thanks a lot for the instruction.
-- 

Best Regards
Qiao