davinci-mcasp: fix tdm_slots and CBM/CFS
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(-)
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
@@ -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
Mark Brown
AppliedRe: [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.
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
PATCH 2/4 davinci-mcasp: use bitfield definitions for PDIR
In reply to Ben GardinerThe 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
@@ -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
PATCH 3/4 davinci-mcasp: fix _CBM_CFS hw_params
In reply to Ben GardinerThe 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
@@ -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
PATCH 4/4 davinci-mcasp: fix _CBM_CFS pin directions
In reply to Ben GardinerThe 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
@@ -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
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: correcttdm_slotslimit
davinci-mcasp: use bitfield definitions for PDIR
davinci-mcasp: fix_CBM_CFShw_params
davinci-mcasp: fix_CBM_CFSpin directionssound/soc/davinci/davinci-mcasp.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-)
All