Skip to contentSkip to conversation

davinci-mcasp: fix tdm_slots and CBM/CFS

PATCH Applied Ben Gardiner 3 participants 8 messages Apr 21, 2011 – Apr 26, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover davinci-mcasp: fix tdm_slots and CBM/CFS

This patch series is comprised of three bugfixes and one cleanup that were performed during prototyping of McASP operation in codec clock- master frame-slave mode.

First we noticed that the check of the number of tdm slots requested by platform data was always returning true -- unrelated to CMB/CFS mode.

Then a cleanup: the PDIR values set are currently based on magic numbers and there are available bitfield definitions. Not strictly needed but it makes the changes introduced in the last patch simpler to read.

It was found that the hardware parameters assigned when codec clock-master frame-slave is requested were incorrect. This change is required for correct CBM/CFS operation.

Finally, the direction of the pins is corrected to reflect the implications of codec clock-master frame-slave -- i.e. mcasp clock- input frame-output. This change is also required for correct operation.

The combination was tested with a logic analyzer and the hrtimer pwm device from Bill Gatliff's PWM framework [1] on a da850evm with hardware modifications to access the McASP lines.

[1] http://article.gmane.org/gmane.linux.kernel.embedded/3486/

Ben Gardiner (4):
davinci-mcasp: correct tdm_slots limit
davinci-mcasp: use bitfield definitions for PDIR
davinci-mcasp: fix _CBM_CFS hw_params
davinci-mcasp: fix _CBM_CFS pin directions

 sound/soc/davinci/davinci-mcasp.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)
#1 lore raw

PATCH 1/4 davinci-mcasp: correct tdm_slots limit

The current check for the number of tdm-slots specified by platform data is always true (x >= 2 || x <= 32); therefore the else branch that warns of an incorrect number of slots can never be taken.

Check that the number of tdm slots specified by platform data is between 2 and 32, inclusive.

---
 sound/soc/davinci/davinci-mcasp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
sound/soc/davinci/davinci-mcasp.c +2 -2
index fb55d2c..e595756 100644
@@ -644,7 +644,7 @@ static void davinci_hw_param(struct davinci_audio_dev *dev, int stream)
 		mcasp_set_reg(dev->base + DAVINCI_MCASP_TXTDM_REG, mask);
 		mcasp_set_bits(dev->base + DAVINCI_MCASP_TXFMT_REG, TXORD);
 
-		if ((dev->tdm_slots >= 2) || (dev->tdm_slots <= 32))
+		if ((dev->tdm_slots >= 2) && (dev->tdm_slots <= 32))
 			mcasp_mod_bits(dev->base + DAVINCI_MCASP_TXFMCTL_REG,
 					FSXMOD(dev->tdm_slots), FSXMOD(0x1FF));
 		else
@@ -660,7 +660,7 @@ static void davinci_hw_param(struct davinci_audio_dev *dev, int stream)
 				AHCLKRE);
 		mcasp_set_reg(dev->base + DAVINCI_MCASP_RXTDM_REG, mask);
 
-		if ((dev->tdm_slots >= 2) || (dev->tdm_slots <= 32))
+		if ((dev->tdm_slots >= 2) && (dev->tdm_slots <= 32))
 			mcasp_mod_bits(dev->base + DAVINCI_MCASP_RXFMCTL_REG,
 					FSRMOD(dev->tdm_slots), FSRMOD(0x1FF));
 		else
Signature · 1 line
1.7.1
Applied
#2 lore raw

Re: [PATCH 1/4] davinci-mcasp: correct tdm_slots limit

Quoted 3 lines · Ben Gardiner

The current check for the number of tdm-slots specified by platform data is always true (x >= 2 || x <= 32); therefore the else branch that warns of an incorrect number of slots can never be taken.

Applied all of these. Please always try to ensure that your commit logs are consistent with the rest of the subsystem so they don't need to be rewritten.

#3 lore raw

Re: [PATCH 1/4] davinci-mcasp: correct tdm_slots limit

Quoted 8 lines · Mark Brown
Quoted 3 lines · Ben Gardiner

The current check for the number of tdm-slots specified by platform data is always true (x >= 2 || x <= 32); therefore the else branch that warns of an incorrect number of slots can never be taken.

Applied all of these.  Please always try to ensure that your commit logs are consistent with the rest of the subsystem so they don't need to be rewritten.

Thanks, Mark, for taking the patches anyways (and Liam for the Ack's) -- Sorry I forgot the 'ASoC' tag (I noticed this patch was committed as 049cfaa ASoC: davinci-mcasp: correct tdm_slots limit).

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

#4 lore raw

PATCH 2/4 davinci-mcasp: use bitfield definitions for PDIR

In reply to Ben Gardiner

The current driver creates value for set/clr of PDIR using (x<<26) instead of the #defines that are convieniently made available.

Update the driver to use the bitfield definitions of PDIR. There is no functional change introduced by this patch.

---
 sound/soc/davinci/davinci-mcasp.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
sound/soc/davinci/davinci-mcasp.c +6 -3
index e595756..1aa24a1 100644
@@ -434,7 +434,8 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		mcasp_set_bits(base + DAVINCI_MCASP_ACLKRCTL_REG, ACLKRE);
 		mcasp_set_bits(base + DAVINCI_MCASP_RXFMCTL_REG, AFSRE);
 
-		mcasp_set_bits(base + DAVINCI_MCASP_PDIR_REG, (0x7 << 26));
+		mcasp_set_bits(base + DAVINCI_MCASP_PDIR_REG,
+				ACLKX | AHCLKX | AFSX);
 		break;
 	case SND_SOC_DAIFMT_CBM_CFS:
 		/* codec is clock master and frame slave */
@@ -444,7 +445,8 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		mcasp_set_bits(base + DAVINCI_MCASP_ACLKRCTL_REG, ACLKRE);
 		mcasp_set_bits(base + DAVINCI_MCASP_RXFMCTL_REG, AFSRE);
 
-		mcasp_set_bits(base + DAVINCI_MCASP_PDIR_REG, (0x2d << 26));
+		mcasp_set_bits(base + DAVINCI_MCASP_PDIR_REG,
+				ACLKX | AFSX | ACLKR | AFSR);
 		break;
 	case SND_SOC_DAIFMT_CBM_CFM:
 		/* codec is clock and frame master */
@@ -454,7 +456,8 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		mcasp_clr_bits(base + DAVINCI_MCASP_ACLKRCTL_REG, ACLKRE);
 		mcasp_clr_bits(base + DAVINCI_MCASP_RXFMCTL_REG, AFSRE);
 
-		mcasp_clr_bits(base + DAVINCI_MCASP_PDIR_REG, (0x3f << 26));
+		mcasp_clr_bits(base + DAVINCI_MCASP_PDIR_REG,
+				ACLKX | AHCLKX | AFSX | ACLKR | AHCLKR | AFSR);
 		break;
 
 	default:
Signature · 1 line
1.7.1
#5 lore raw

PATCH 3/4 davinci-mcasp: fix _CBM_CFS hw_params

In reply to Ben Gardiner

The current davinci_mcasp_set_dai_fmt() sets bits ACLKXE and ACLKRE (CLKXM and CLKRM as they are reffered to in SPRUFM1 [1]) for codec clock-slave/ frame-slave mode (_CBS_CFS) which selects internally generated bit-clock and frame-sync signals; however, it does the same thing again for codec clock-master/frame-slave mode (_CBM_CFS) in the very next case statement which is incorrectly selecting internally generated bit-clocks in this mode.

For codec clock-master/frame-slave mode (_CBM_CFS), clear bits ACLKXE and ACLKRE to select externally-generated bit-clocks.

[1] http://www.ti.com/litv/pdf/sprufm1

---
 sound/soc/davinci/davinci-mcasp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
sound/soc/davinci/davinci-mcasp.c +2 -2
index 1aa24a1..8004643 100644
@@ -439,10 +439,10 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		break;
 	case SND_SOC_DAIFMT_CBM_CFS:
 		/* codec is clock master and frame slave */
-		mcasp_set_bits(base + DAVINCI_MCASP_ACLKXCTL_REG, ACLKXE);
+		mcasp_clr_bits(base + DAVINCI_MCASP_ACLKXCTL_REG, ACLKXE);
 		mcasp_set_bits(base + DAVINCI_MCASP_TXFMCTL_REG, AFSXE);
 
-		mcasp_set_bits(base + DAVINCI_MCASP_ACLKRCTL_REG, ACLKRE);
+		mcasp_clr_bits(base + DAVINCI_MCASP_ACLKRCTL_REG, ACLKRE);
 		mcasp_set_bits(base + DAVINCI_MCASP_RXFMCTL_REG, AFSRE);
 
 		mcasp_set_bits(base + DAVINCI_MCASP_PDIR_REG,
Signature · 1 line
1.7.1
#6 lore raw

PATCH 4/4 davinci-mcasp: fix _CBM_CFS pin directions

In reply to Ben Gardiner

The current davinci_mcasp_set_dai_fmt() sets bits ACLKX and ACLKR in the PDIR register for the codec clock-master/frame-slave mode; however, this results in the ACLKX and ACLKR pins being outputs according to SPRUFM1 [1] which conflicts with "codec is clock master."

Similarly to the previous patch in this series, "fix _CBM_CFS hw_params" -- For codec clock-master/frame-slave mode (_CMB_CFS), clear bits ACLKX and ACLKR in the PDIR register to set the pins as inputs and hence allow externally sourced bit-clocks.

[1] http://www.ti.com/litv/pdf/sprufm1

---
 sound/soc/davinci/davinci-mcasp.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
sound/soc/davinci/davinci-mcasp.c +3 -1
index 8004643..0a3d891 100644
@@ -445,8 +445,10 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		mcasp_clr_bits(base + DAVINCI_MCASP_ACLKRCTL_REG, ACLKRE);
 		mcasp_set_bits(base + DAVINCI_MCASP_RXFMCTL_REG, AFSRE);
 
+		mcasp_clr_bits(base + DAVINCI_MCASP_PDIR_REG,
+				ACLKX | ACLKR);
 		mcasp_set_bits(base + DAVINCI_MCASP_PDIR_REG,
-				ACLKX | AFSX | ACLKR | AFSR);
+				AFSX | AFSR);
 		break;
 	case SND_SOC_DAIFMT_CBM_CFM:
 		/* codec is clock and frame master */
Signature · 1 line
1.7.1
Acked-by on the v1 series
#7 lore raw
In reply to Ben Gardiner
Quoted 27 lines · Ben Gardiner

This patch series is comprised of three bugfixes and one cleanup that were performed during prototyping of McASP operation in codec clock- master frame-slave mode.

First we noticed that the check of the number of tdm slots requested by platform data was always returning true -- unrelated to CMB/CFS mode.

Then a cleanup: the PDIR values set are currently based on magic numbers and there are available bitfield definitions. Not strictly needed but it makes the changes introduced in the last patch simpler to read.

It was found that the hardware parameters assigned when codec clock-master frame-slave is requested were incorrect. This change is required for correct CBM/CFS operation.

Finally, the direction of the pins is corrected to reflect the implications of codec clock-master frame-slave -- i.e. mcasp clock- input frame-output. This change is also required for correct operation.

The combination was tested with a logic analyzer and the hrtimer pwm device from Bill Gatliff's PWM framework [1] on a da850evm with hardware modifications to access the McASP lines.

[1] http://article.gmane.org/gmane.linux.kernel.embedded/3486/

Quoted 9 lines

Ben Gardiner (4):
davinci-mcasp: correct tdm_slots limit
davinci-mcasp: use bitfield definitions for PDIR
davinci-mcasp: fix _CBM_CFS hw_params
davinci-mcasp: fix _CBM_CFS pin directions

 sound/soc/davinci/davinci-mcasp.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

All