[alsa-devel] [PATCH 0/4] 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(-)
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.
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca Reviewed-by: James Nuss jamesnuss@nanometrics.ca --- sound/soc/davinci/davinci-mcasp.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index fb55d2c..e595756 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -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
On Thu, Apr 21, 2011 at 02:19:01PM -0400, Ben Gardiner wrote:
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.
On Tue, Apr 26, 2011 at 6:46 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Thu, Apr 21, 2011 at 02:19:01PM -0400, Ben Gardiner wrote:
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
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.
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca Reviewed-by: James Nuss jamesnuss@nanometrics.ca --- sound/soc/davinci/davinci-mcasp.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index e595756..1aa24a1 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -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:
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
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca Reviewed-by: James Nuss jamesnuss@nanometrics.ca --- sound/soc/davinci/davinci-mcasp.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index 1aa24a1..8004643 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -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,
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
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca Reviewed-by: James Nuss jamesnuss@nanometrics.ca --- sound/soc/davinci/davinci-mcasp.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index 8004643..0a3d891 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -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 */
On Thu, 2011-04-21 at 14:19 -0400, Ben Gardiner wrote:
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(-)
All
Acked-by: Liam Girdwood lrg@ti.com
participants (3)
-
Ben Gardiner -
Liam Girdwood -
Mark Brown