On 03/27/2014 10:39 AM, Daniel Mack wrote:
Hi Peter,
Thanks for your review.
On 03/27/2014 09:16 AM, Peter Ujfalusi wrote:
On 03/26/2014 05:04 PM, Daniel Mack wrote:
In DIT (S/PDIF) mode, program the transmitted user bits to reflect the configured sample rate, along with some other details.
Signed-off-by: Daniel Mack zonque@gmail.com
sound/soc/davinci/davinci-mcasp.c | 55 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index 712a7cd..ec0463a 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -27,6 +27,7 @@ #include <linux/of_platform.h> #include <linux/of_device.h>
+#include <sound/asoundef.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -566,8 +567,11 @@ static int mcasp_i2s_hw_param(struct davinci_mcasp *mcasp, int stream) }
/* S/PDIF */ -static int mcasp_dit_hw_param(struct davinci_mcasp *mcasp) +static int mcasp_dit_hw_param(struct davinci_mcasp *mcasp,
unsigned int rate){
- u32 val = 0;
- /* Set the TX format : 24 bit right rotation, 32 bit slot, Pad 0 and LSB first */ mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMT_REG, TXROT(6) | TXSSZ(15));
@@ -589,6 +593,53 @@ static int mcasp_dit_hw_param(struct davinci_mcasp *mcasp) /* Enable the DIT */ mcasp_set_bits(mcasp, DAVINCI_MCASP_TXDITCTL_REG, DITEN);
- /* Set S/PDIF channel status bits */
- mcasp_set_reg(mcasp, DAVINCI_MCASP_DITCSRA_REG + 0,
IEC958_AES0_CON_NOT_COPYRIGHT);I think it would be safer to set the channel status bits like: u32 val = 0; u8 *bytes = (u8*) &val;
byte[0] |= IEC958_AES0_CON_NOT_COPYRIGHT; byte[1] |= IEC958_AES1_CON_PCM_CODER;
No, these defines are mapped on to 32-bit values, as seen in include/sound/asounddef.h. Over all 6 registers, 192 bits can be stored, which is the full length of channel status bits. Hence, they really need an individual mcasp_set_reg() each.
I don't think they are mapped for the 32bit: #define IEC958_AES0_CON_NOT_COPYRIGHT (1<<2)
#define IEC958_AES1_CON_DIGDIGCONV_ID 0x02 #define IEC958_AES1_CON_PCM_CODER (IEC958_AES1_CON_DIGDIGCONV_ID|0x00)
#define IEC958_AES3_CON_FS_48000 (2<<0) /* 48kHz */ #define IEC958_AES3_CON_FS_32000 (3<<0) /* 32kHz */ #define IEC958_AES3_CON_FS_22050 (4<<0) /* 22.05kHz */ #define IEC958_AES3_CON_FS_24000 (6<<0) /* 24kHz */
From the AES3 spec (byte numbers are 1 based while bit numbers are 0 based for
some reason in the specs): Copyright bit is on Byte1's bit 2 Byte2 has the category code and Byte4's 0-3 bits for the sampling frequency.
All of the setting you are changing ar in the first 32bit of the whole channels status array, which is 24 byte long.
Also, I've measured the output of the S/PDIF port with external S/PDIF introspectors and the result is correct :)
Well you essentially writing to the correct byte offset but I don't think in a correct way:
write to DAVINCI_MCASP_DITCSRA_REG + 0 is a write starting at Byte1 write to DAVINCI_MCASP_DITCSRA_REG + 1 is a write starting at Byte2 write to DAVINCI_MCASP_DITCSRA_REG + 3 is a write starting at Byte4
The interconnect I think is clever enough to figure out that you try to write to non 32bit boundary and does corrects the access (shifting the data to it's correct place) but I don't think we should rely on that. It is much simpler to configure the bits first and write it as one 32bit.