Re: [PATCH INTERNAL 1/1] ASoC: cygnus: Remove set_fmt from SPDIF dai ops
On 9/28/2017 3:29 PM, Lori Hikichi wrote:
> The SPDIF port cannot modify its format so a set_fmt function is not
> needed. Previously, we used a generic set_fmt for all ports and returned
> an error code for the SPDIF port. It is cleaner to not populate the
> set_fmt field.
>
> Signed-off-by: Lori Hikichi
> ---
> sound/soc/bcm/cygnus-ssp.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
> index 15c438f..b9a3bb8d 100644
> --- a/sound/soc/bcm/cygnus-ssp.c
> +++ b/sound/soc/bcm/cygnus-ssp.c
> @@ -1136,6 +1136,13 @@ static int cygnus_ssp_resume(struct snd_soc_dai
> *cpu_dai)
> .set_tdm_slot = cygnus_set_dai_tdm_slot,
> };
>
> +static const struct snd_soc_dai_ops cygnus_spdif_dai_ops = {
> + .startup= cygnus_ssp_startup,
> + .shutdown = cygnus_ssp_shutdown,
> + .trigger= cygnus_ssp_trigger,
> + .hw_params = cygnus_ssp_hw_params,
> + .set_sysclk = cygnus_ssp_set_sysclk,
> +};
>
> #define INIT_CPU_DAI(num) { \
> .name = "cygnus-ssp" #num, \
> @@ -1174,7 +1181,7 @@ static int cygnus_ssp_resume(struct snd_soc_dai
> *cpu_dai)
> .formats = SNDRV_PCM_FMTBIT_S16_LE |
> SNDRV_PCM_FMTBIT_S32_LE,
> },
> - .ops = &cygnus_ssp_dai_ops,
> + .ops = &cygnus_spdif_dai_ops,
> .suspend = cygnus_ssp_suspend,
> .resume = cygnus_ssp_resume,
> };
Please ignore this patch. It was accidentally included in the patch set and is a
duplicate of another patch already in the patch set.
[PATCH v1 2/3] ASoC: cygnus: Remove set_fmt from SPDIF dai ops
The SPDIF port cannot modify its format so a set_fmt function is not
needed. Previously, we used a generic set_fmt for all ports and returned
an error code for the SPDIF port. It is cleaner to not populate the
set_fmt field.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index e9c73a4..da14fac 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -1137,6 +1137,13 @@ static int cygnus_ssp_resume(struct snd_soc_dai *cpu_dai)
.set_tdm_slot = cygnus_set_dai_tdm_slot,
};
+static const struct snd_soc_dai_ops cygnus_spdif_dai_ops = {
+ .startup= cygnus_ssp_startup,
+ .shutdown = cygnus_ssp_shutdown,
+ .trigger= cygnus_ssp_trigger,
+ .hw_params = cygnus_ssp_hw_params,
+ .set_sysclk = cygnus_ssp_set_sysclk,
+};
#define INIT_CPU_DAI(num) { \
.name = "cygnus-ssp" #num, \
@@ -1175,7 +1182,7 @@ static int cygnus_ssp_resume(struct snd_soc_dai *cpu_dai)
.formats = SNDRV_PCM_FMTBIT_S16_LE |
SNDRV_PCM_FMTBIT_S32_LE,
},
- .ops = &cygnus_ssp_dai_ops,
+ .ops = &cygnus_spdif_dai_ops,
.suspend = cygnus_ssp_suspend,
.resume = cygnus_ssp_resume,
};
--
1.9.1
[PATCH v1 1/3] ASoC: cygnus: Add EXPORT_SYMBOL for helper function
The helper function cygnus_ssp_set_custom_fsync_width() is intended
to be called from an ASoC machine driver, need to export symbol
if using modules.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index 15c438f..e9c73a4 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -842,6 +842,7 @@ int cygnus_ssp_set_custom_fsync_width(struct snd_soc_dai
*cpu_dai, int len)
return -EINVAL;
}
}
+EXPORT_SYMBOL_GPL(cygnus_ssp_set_custom_fsync_width);
static int cygnus_ssp_set_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
{
--
1.9.1
[PATCH INTERNAL 1/1] ASoC: cygnus: Remove set_fmt from SPDIF dai ops
The SPDIF port cannot modify its format so a set_fmt function is not
needed. Previously, we used a generic set_fmt for all ports and returned
an error code for the SPDIF port. It is cleaner to not populate the
set_fmt field.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index 15c438f..b9a3bb8d 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -1136,6 +1136,13 @@ static int cygnus_ssp_resume(struct snd_soc_dai *cpu_dai)
.set_tdm_slot = cygnus_set_dai_tdm_slot,
};
+static const struct snd_soc_dai_ops cygnus_spdif_dai_ops = {
+ .startup= cygnus_ssp_startup,
+ .shutdown = cygnus_ssp_shutdown,
+ .trigger= cygnus_ssp_trigger,
+ .hw_params = cygnus_ssp_hw_params,
+ .set_sysclk = cygnus_ssp_set_sysclk,
+};
#define INIT_CPU_DAI(num) { \
.name = "cygnus-ssp" #num, \
@@ -1174,7 +1181,7 @@ static int cygnus_ssp_resume(struct snd_soc_dai *cpu_dai)
.formats = SNDRV_PCM_FMTBIT_S16_LE |
SNDRV_PCM_FMTBIT_S32_LE,
},
- .ops = &cygnus_ssp_dai_ops,
+ .ops = &cygnus_spdif_dai_ops,
.suspend = cygnus_ssp_suspend,
.resume = cygnus_ssp_resume,
};
--
1.9.1
[PATCH v1 3/3] ASoC: cygnus: Remove support for 8 bit audio and for mono
These modes of operation were not working properly and it is
unclear if the hardware could fully support these modes properly.
There is little to be gained by enabling these modes, therefore,
we will just remove support.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 22 --
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index da14fac..cd8aef8 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -655,23 +655,10 @@ static int cygnus_ssp_hw_params(struct snd_pcm_substream
*substream,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
value = readl(aio->cygaud->audio + aio->regs.bf_sourcech_cfg);
value &= ~BIT(BF_SRC_CFGX_BUFFER_PAIR_ENABLE);
- /* Configure channels as mono or stereo/TDM */
- if (params_channels(params) == 1)
- value |= BIT(BF_SRC_CFGX_SAMPLE_CH_MODE);
- else
- value &= ~BIT(BF_SRC_CFGX_SAMPLE_CH_MODE);
+ value &= ~BIT(BF_SRC_CFGX_SAMPLE_CH_MODE);
writel(value, aio->cygaud->audio + aio->regs.bf_sourcech_cfg);
switch (params_format(params)) {
- case SNDRV_PCM_FORMAT_S8:
- if (aio->port_type == PORT_SPDIF) {
- dev_err(aio->cygaud->dev,
- "SPDIF does not support 8bit format\n");
- return -EINVAL;
- }
- bitres = 8;
- break;
-
case SNDRV_PCM_FORMAT_S16_LE:
bitres = 16;
break;
@@ -1148,11 +1135,10 @@ static int cygnus_ssp_resume(struct snd_soc_dai
*cpu_dai)
#define INIT_CPU_DAI(num) { \
.name = "cygnus-ssp" #num, \
.playback = { \
- .channels_min = 1, \
+ .channels_min = 2, \
.channels_max = 16, \
.rates = SNDRV_PCM_RATE_KNOT, \
- .formats = SNDRV_PCM_FMTBIT_S8 | \
- SNDRV_PCM_FMTBIT_S16_LE | \
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | \
SNDRV_PCM_FMTBIT_S32_LE, \
}, \
.capture = { \
@@ -1160,7 +1146,7 @@ static int cygnus_ssp_resume(struct snd_soc_dai *cpu_dai)
.channels_max = 16, \
.rates = SNDRV_PCM_RATE_KNOT, \
.formats = SNDRV_PCM_FMTBIT_S16_LE | \
- SNDRV_PCM_FMTBIT_S32_LE, \
+ SNDRV_PCM_FMTBIT_S32_LE, \
}, \
.ops = &cygnus_ssp_dai_ops, \
.suspend = cygnus_ssp_suspend, \
--
1.9.1
[PATCH v1 0/3] Minor updates to driver
This patchset contains a few small updates to the DAI driver. Lori Hikichi (3): ASoC: cygnus: Add EXPORT_SYMBOL for helper function ASoC: cygnus: Remove set_fmt from SPDIF dai ops ASoC: cygnus: Remove support for 8 bit audio and for mono sound/soc/bcm/cygnus-ssp.c | 32 +--- 1 file changed, 13 insertions(+), 19 deletions(-) -- 1.9.1
Re: [PATCH 2/9] ASoC: cygnus: Update bindings for audio clock changes
On 8/22/2017 9:07 AM, Mark Brown wrote: > On Wed, Aug 16, 2017 at 12:39:42PM -0700, Lori Hikichi wrote: > >> By far the most common usage case for Cygnus is a configuration which >> uses only the three i2s/tdm ports. In this case each port is assigned >> a clock. Each clock has the same capabilities so there would never be >> a reason change the static mapping. > The usual reason would be to bring things into sync. > >> Now for the case when the "non-audio block" uses one of these clocks. >> In this situation we will only need one i2s port because this >> configuration of the chip is not audio intensive. When the system >> is designed we know if this non-audio block will be in use, it is not >> a runtime configurable thing. Again, a static mapping is fine. > Is this limitation when the other block is in use a physical limitation > or is it just a case of not seeing the use case. > >> At the time it did not seem necessary to make addition driver changes to >> support a use case that will very likely never arise. As it turns out, >> we are working on a new version of this audio block. The clocking >> configuration for this new version is more complex and I am already in >> the process of creating clock bindings for all this this. I am hopeful >> that the driver for this new version will be applicable to Cygnus. > If the clocking is more complex that seems like even more reason to not > fix this in the binding, and possibly to do as I think I suggested > earlier and use the common clock bindings to manage this rather than > doing something custom and driver specific. Ok, I will create the necessary clocks to allow this to work. This will likely impact a couple of the other patches in the series, but most should be applicable. What would be the best way to have some of the other patches in this series reviewed? Should I just drop this series and resubmit new individual patches for review.
Re: [PATCH 2/9] ASoC: cygnus: Update bindings for audio clock changes
On 8/16/2017 3:59 AM, Mark Brown wrote: > On Tue, Aug 15, 2017 at 12:29:44PM -0700, Lori Hikichi wrote: > >> I have put the mux assignment in DT because the assignment is a >> static property and did not need run time programmability from the >> machine driver. > Why is this a static property, what prevents something wanting to change > things at runtime? You might be running with simpler setups now but > perhaps you'll run into a more complex use case later? The short answer is I have analyzed the possible use cases for Cygnus' audio block, and nothing should need to change the assignments at runtime. The longer explanation follows. The clocking configuration is this. There is one pll with its output run through 3 post dividers. The audio ports can select one of these outputs. There are only 5 possible consumers of these 3 clocks. The 3 i2s/tdm ports, 1 spdif port, and an exceptional case of another "non-audio" IP block. For the i2s/tdm ports this clock is the MCLK. By far the most common usage case for Cygnus is a configuration which uses only the three i2s/tdm ports. In this case each port is assigned a clock. Each clock has the same capabilities so there would never be a reason change the static mapping. Now for the case when the "non-audio block" uses one of these clocks. In this situation we will only need one i2s port because this configuration of the chip is not audio intensive. When the system is designed we know if this non-audio block will be in use, it is not a runtime configurable thing. Again, a static mapping is fine. The only situation which could get more complex is with SPDIF. First off, the SPDIF port is not actively used in any current configuration and I do not think there are any plans for it to be used. But, we are talking about possible future configurations. The only limitation the current static scheme would introduce is if SPDIF is active along with all 3 i2s ports. Additionally, all 3 of the i2s ports would need to be in master mode (slave mode would free up a clock for SPDIF). In this case, two of the clock consumers would need to share a clock. In this situation I envisioned that both consumers would agree on a fixed rate and work within those limitation. For example, the ports would choose 24.576 MHz as their mclk and be limited to the the frame rates that could be derived from that clock. At the time it did not seem necessary to make addition driver changes to support a use case that will very likely never arise. As it turns out, we are working on a new version of this audio block. The clocking configuration for this new version is more complex and I am already in the process of creating clock bindings for all this this. I am hopeful that the driver for this new version will be applicable to Cygnus. Lori.
Re: [PATCH 2/9] ASoC: cygnus: Update bindings for audio clock changes
On 8/15/2017 10:14 AM, Mark Brown wrote: > On Mon, Aug 14, 2017 at 03:06:50PM -0700, Lori Hikichi wrote: >> Allow each audio port to select which clock (if any) it wants to use. > Why is this in DT for the port and not either using standard clock > bindings to configure the clock tree or allowing the machine driver to > pick? The previous version of the driver essentially had a clock mapping that could not be changed. This is fine for 99% of our use cases. If we need to change the mapping, then we need to modify the audio port's clock mux. Creating a clock for these muxes was going to be messy. There is a mux per audio port and the registers used to program the muxes are staggered throughout the io space used by the audio driver. Additionally, these registers have bits that are controlled by the audio driver. Had I used syscon to access these registers this would have resulted in a very fragmented io space, complicating the audio drivers access this space. I have put the mux assignment in DT because the assignment is a static property and did not need run time programmability from the machine driver. Lori.
[PATCH 1/9] ASoC: cygnus: Add support for 384kHz frame rates
Allow the audio ports to operate at 384kHz.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 235 +++--
1 file changed, 55 insertions(+), 180 deletions(-)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index e710bb0..1a57a4e 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -27,12 +27,6 @@
#define DEFAULT_VCO1354750204
-#define CYGNUS_TDM_RATE \
- (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 | \
- SNDRV_PCM_RATE_11025 | SNDRV_PCM_RATE_22050 | \
- SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \
- SNDRV_PCM_RATE_48000)
-
#define CAPTURE_FCI_ID_BASE 0x180
#define CYGNUS_SSP_TRISTATE_MASK 0x001fff
#define CYGNUS_PLLCLKSEL_MASK 0xf
@@ -234,152 +228,20 @@ struct pll_macro_entry {
{98304000, 2},
};
+#define CYGNUS_RATE_MIN 8000
+#define CYGNUS_RATE_MAX 384000
+
/* List of valid frame sizes for tdm mode */
static const int ssp_valid_tdm_framesize[] = {32, 64, 128, 256, 512};
-/*
- * Use this relationship to derive the sampling rate (lrclk)
- * lrclk = (mclk) / ((2*mclk_to_sclk_ratio) * (32 * SCLK))).
- *
- * Use mclk and pll_ch from the table above
- *
- * Valid SCLK = 0/1/2/4/8/12
- *
- * mclk_to_sclk_ratio = number of MCLK per SCLK. Division is twice the
- * value programmed in this field.
- * Valid mclk_to_sclk_ratio = 1 through to 15
- *
- * eg: To set lrclk = 48khz, set mclk = 12288000, mclk_to_sclk_ratio = 2,
- * SCLK = 64
- */
-struct _ssp_clk_coeff {
- u32 mclk;
- u32 sclk_rate;
- u32 rate;
- u32 mclk_rate;
+static const unsigned int cygnus_rates[] = {
+8000, 11025, 16000, 22050, 32000, 44100, 48000,
+ 88200, 96000, 176400, 192000, 352800, 384000
};
-static const struct _ssp_clk_coeff ssp_clk_coeff[] = {
- { 4096000, 32, 16000, 4},
- { 4096000, 32, 32000, 2},
- { 4096000, 64, 8000, 4},
- { 4096000, 64, 16000, 2},
- { 4096000, 64, 32000, 1},
- { 4096000, 128, 8000, 2},
- { 4096000, 128, 16000, 1},
- { 4096000, 256, 8000, 1},
-
- { 6144000, 32, 16000, 6},
- { 6144000, 32, 32000, 3},
- { 6144000, 32, 48000, 2},
- { 6144000, 32, 96000, 1},
- { 6144000, 64, 8000, 6},
- { 6144000, 64, 16000, 3},
- { 6144000, 64, 48000, 1},
- { 6144000, 128, 8000, 3},
-
- { 8192000, 32, 32000, 4},
- { 8192000, 64, 16000, 4},
- { 8192000, 64, 32000, 2},
- { 8192000, 128, 8000, 4},
- { 8192000, 128, 16000, 2},
- { 8192000, 128, 32000, 1},
- { 8192000, 256, 8000, 2},
- { 8192000, 256, 16000, 1},
- { 8192000, 512, 8000, 1},
-
- {12288000, 32, 32000, 6},
- {12288000, 32, 48000, 4},
- {12288000, 32, 96000, 2},
- {12288000, 32, 192000, 1},
- {12288000, 64, 16000, 6},
- {12288000, 64, 32000, 3},
- {12288000, 64, 48000, 2},
- {12288000, 64, 96000, 1},
- {12288000, 128, 8000, 6},
- {12288000, 128, 16000, 3},
- {12288000, 128, 48000, 1},
- {12288000, 256, 8000, 3},
-
- {16384000, 64, 32000, 4},
- {16384000, 128, 16000, 4},
- {16384000, 128, 32000, 2},
- {16384000, 256, 8000, 4},
- {16384000, 256, 16000, 2},
- {16384000, 256, 32000, 1},
- {16384000, 512, 8000, 2},
- {16384000, 512, 16000, 1},
-
- {24576000, 32, 96000, 4},
- {24576000, 32, 192000, 2},
- {24576000, 64, 32000, 6},
- {24576000, 64, 48000, 4},
- {24576000, 64, 96000, 2},
- {24576000, 64, 192000, 1},
- {24576000, 128, 16000, 6},
- {24576000, 128, 32000, 3},
- {24576000, 128, 48000, 2},
- {24576000, 256, 8000, 6},
- {24576000, 256, 16000, 3},
- {24576000, 256, 48000, 1},
- {24576000, 512, 8000, 3},
-
- {49152000, 32, 192000, 4},
- {49152000, 64, 96000, 4},
- {49152000, 64, 192000, 2},
- {49152000, 128, 32000, 6},
- {49152000, 128, 48000, 4},
- {49152000, 128, 96000, 2},
- {49152000, 128, 192000, 1},
- {49152000, 256, 16000, 6},
- {49152000, 256, 32000, 3},
- {49152000, 256, 48000, 2},
- {49152000, 256, 96000, 1},
- {49152000, 512, 8000, 6},
- {49152000, 512, 16000, 3},
- {49152000, 512, 48000, 1},
-
- { 5644800, 32, 22050, 4},
- { 5644800, 32, 44100, 2},
- { 5644800, 32, 88200, 1},
- { 5644800, 64, 11025, 4},
- { 5644800, 64, 22050, 2},
- { 5644800, 64, 44100, 1},
-
- {11289600, 32, 44100, 4},
- {11289600, 32, 88200, 2},
- {11289600, 32, 176400, 1},
- {11289600, 64, 22050, 4},
- {11289600, 64, 44100, 2},
- {11289600, 64, 88200, 1},
- {11289600, 128, 11025, 4},
- {11289600, 128, 22050, 2},
- {11289600
[PATCH 3/9] ASoC: cygnus: Allow each port to select its clock source
Add the ability to assign which of the 3 audio PLL outputs are to be
used by each port. Remove the suspend and resume handlers because the only
thing they were doing was unnecessarily maintaining the clock state.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 332 ++---
sound/soc/bcm/cygnus-ssp.h | 15 +-
2 files changed, 103 insertions(+), 244 deletions(-)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index 1a57a4e..00fd4dc 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -25,8 +25,6 @@
#include "cygnus-ssp.h"
-#define DEFAULT_VCO1354750204
-
#define CAPTURE_FCI_ID_BASE 0x180
#define CYGNUS_SSP_TRISTATE_MASK 0x001fff
#define CYGNUS_PLLCLKSEL_MASK 0xf
@@ -95,22 +93,10 @@
#define SPDIF_FORMAT_CFG_OFFSET 0xad8
#define SPDIF_MCLK_CFG_OFFSET0xadc
-/* AUD_FMM_IOP_PLL_0_xxx regs */
-#define IOP_PLL_0_MACRO_OFFSET0xb00
-#define IOP_PLL_0_MDIV_Ch0_OFFSET 0xb14
-#define IOP_PLL_0_MDIV_Ch1_OFFSET 0xb18
-#define IOP_PLL_0_MDIV_Ch2_OFFSET 0xb1c
-
-#define IOP_PLL_0_ACTIVE_MDIV_Ch0_OFFSET 0xb30
-#define IOP_PLL_0_ACTIVE_MDIV_Ch1_OFFSET 0xb34
-#define IOP_PLL_0_ACTIVE_MDIV_Ch2_OFFSET 0xb38
-
-/* AUD_FMM_IOP_xxx regs */
-#define IOP_PLL_0_CONTROL_OFFSET 0xb04
-#define IOP_PLL_0_USER_NDIV_OFFSET 0xb08
-#define IOP_PLL_0_ACTIVE_NDIV_OFFSET 0xb20
-#define IOP_PLL_0_RESET_OFFSET 0xb5c
+/*
+ * Register offsets for i2s_in io space
+ */
/* AUD_FMM_IOP_IN_I2S_xxx regs */
#define IN_I2S_0_STREAM_CFG_OFFSET 0x00
#define IN_I2S_0_CFG_OFFSET0x04
@@ -173,12 +159,6 @@
#define SPDIF_0_OUT_DITHER_ENA 3
#define SPDIF_0_OUT_STREAM_ENA31
-/* AUD_FMM_IOP_PLL_0_USER */
-#define IOP_PLL_0_USER_NDIV_FRAC 10
-
-/* AUD_FMM_IOP_PLL_0_ACTIVE */
-#define IOP_PLL_0_ACTIVE_NDIV_FRAC 10
-
#define INIT_SSP_REGS(num) (struct cygnus_ssp_regs){ \
.i2s_stream_cfg = OUT_I2S_ ##num## _STREAM_CFG_OFFSET, \
@@ -193,41 +173,6 @@
.bf_sourcech_grp = BF_SRC_GRP ##num## _OFFSET \
}
-struct pll_macro_entry {
- u32 mclk;
- u32 pll_ch_num;
-};
-
-/*
- * PLL has 3 output channels (1x, 2x, and 4x). Below are
- * the common MCLK frequencies used by audio driver
- */
-static const struct pll_macro_entry pll_predef_mclk[] = {
- { 4096000, 0},
- { 8192000, 1},
- {16384000, 2},
-
- { 5644800, 0},
- {11289600, 1},
- {22579200, 2},
-
- { 6144000, 0},
- {12288000, 1},
- {24576000, 2},
-
- {12288000, 0},
- {24576000, 1},
- {49152000, 2},
-
- {22579200, 0},
- {45158400, 1},
- {90316800, 2},
-
- {24576000, 0},
- {49152000, 1},
- {98304000, 2},
-};
-
#define CYGNUS_RATE_MIN 8000
#define CYGNUS_RATE_MAX 384000
@@ -488,59 +433,6 @@ static int audio_ssp_out_disable(struct cygnus_aio_port
*aio)
return status;
}
-static int pll_configure_mclk(struct cygnus_audio *cygaud, u32 mclk,
- struct cygnus_aio_port *aio)
-{
- int i = 0, error;
- bool found = false;
- const struct pll_macro_entry *p_entry;
- struct clk *ch_clk;
-
- for (i = 0; i < ARRAY_SIZE(pll_predef_mclk); i++) {
- p_entry = &pll_predef_mclk[i];
- if (p_entry->mclk == mclk) {
- found = true;
- break;
- }
- }
- if (!found) {
- dev_err(cygaud->dev,
- "%s No valid mclk freq (%u) found!\n", __func__, mclk);
- return -EINVAL;
- }
-
- ch_clk = cygaud->audio_clk[p_entry->pll_ch_num];
-
- if ((aio->clk_trace.cap_en) && (!aio->clk_trace.cap_clk_en)) {
- error = clk_prepare_enable(ch_clk);
- if (error) {
- dev_err(cygaud->dev, "%s clk_prepare_enable failed
%d\n",
- __func__, error);
- return error;
- }
- aio->clk_trace.cap_clk_en = true;
- }
-
- if ((aio->clk_trace.play_en) && (!aio->clk_trace.play_clk_en)) {
- error = clk_prepare_enable(ch_clk);
- if (error) {
- dev_err(cygaud->dev, "%s clk_prepare_enable failed
%d\n",
- __func__, error);
- return error;
- }
- aio->clk_trace.play_clk_en = true;
- }
-
- error = clk_set_rate(ch_clk, mclk);
- if (error) {
- dev_err(cygaud->dev, "%s Set MCLK rate failed: %d\n",
- __func__, error);
- return error;
- }
-
- return p_entry->pll_ch_num;
-}
-
static int cygnus_ssp_set_clocks(struct cygnus_aio_port *aio)
{
u32 value;
@@ -723,26 +615,68
[PATCH 9/9] ASoC: cygnus: Tidy up of structure access
Adds copies of the frequently accessed io handles to each ports
data structure, making it more convenient to access. Also, a small
cleanup to the type names used in cygnus_pcm. None of this should
result in a functional change to the driver.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-pcm.c | 50
sound/soc/bcm/cygnus-ssp.c | 298 +
sound/soc/bcm/cygnus-ssp.h | 22 ++--
3 files changed, 178 insertions(+), 192 deletions(-)
diff --git a/sound/soc/bcm/cygnus-pcm.c b/sound/soc/bcm/cygnus-pcm.c
index d616e096..d82bf55 100644
--- a/sound/soc/bcm/cygnus-pcm.c
+++ b/sound/soc/bcm/cygnus-pcm.c
@@ -329,24 +329,24 @@ static void enable_intr(struct snd_pcm_substream
*substream)
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
/* Clear interrupt status before enabling them */
- writel(clear_mask, aio->cygaud->audio + ESR0_STATUS_CLR_OFFSET);
- writel(clear_mask, aio->cygaud->audio + ESR1_STATUS_CLR_OFFSET);
- writel(clear_mask, aio->cygaud->audio + ESR3_STATUS_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR0_STATUS_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR1_STATUS_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR3_STATUS_CLR_OFFSET);
/* Unmask the interrupts of the given port*/
- writel(clear_mask, aio->cygaud->audio + ESR0_MASK_CLR_OFFSET);
- writel(clear_mask, aio->cygaud->audio + ESR1_MASK_CLR_OFFSET);
- writel(clear_mask, aio->cygaud->audio + ESR3_MASK_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR0_MASK_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR1_MASK_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR3_MASK_CLR_OFFSET);
writel(ANY_PLAYBACK_IRQ,
- aio->cygaud->audio + INTH_R5F_MASK_CLEAR_OFFSET);
+ aio->audio + INTH_R5F_MASK_CLEAR_OFFSET);
} else {
- writel(clear_mask, aio->cygaud->audio + ESR2_STATUS_CLR_OFFSET);
- writel(clear_mask, aio->cygaud->audio + ESR4_STATUS_CLR_OFFSET);
- writel(clear_mask, aio->cygaud->audio + ESR2_MASK_CLR_OFFSET);
- writel(clear_mask, aio->cygaud->audio + ESR4_MASK_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR2_STATUS_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR4_STATUS_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR2_MASK_CLR_OFFSET);
+ writel(clear_mask, aio->audio + ESR4_MASK_CLR_OFFSET);
writel(ANY_CAPTURE_IRQ,
- aio->cygaud->audio + INTH_R5F_MASK_CLEAR_OFFSET);
+ aio->audio + INTH_R5F_MASK_CLEAR_OFFSET);
}
}
@@ -366,12 +366,12 @@ static void disable_intr(struct snd_pcm_substream
*substream)
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
/* Mask the interrupts of the given port*/
- writel(set_mask, aio->cygaud->audio + ESR0_MASK_SET_OFFSET);
- writel(set_mask, aio->cygaud->audio + ESR1_MASK_SET_OFFSET);
- writel(set_mask, aio->cygaud->audio + ESR3_MASK_SET_OFFSET);
+ writel(set_mask, aio->audio + ESR0_MASK_SET_OFFSET);
+ writel(set_mask, aio->audio + ESR1_MASK_SET_OFFSET);
+ writel(set_mask, aio->audio + ESR3_MASK_SET_OFFSET);
} else {
- writel(set_mask, aio->cygaud->audio + ESR2_MASK_SET_OFFSET);
- writel(set_mask, aio->cygaud->audio + ESR4_MASK_SET_OFFSET);
+ writel(set_mask, aio->audio + ESR2_MASK_SET_OFFSET);
+ writel(set_mask, aio->audio + ESR4_MASK_SET_OFFSET);
}
}
@@ -415,13 +415,13 @@ static void cygnus_pcm_period_elapsed(struct
snd_pcm_substream *substream)
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
/* Set the ring buffer to full */
- regval = readl(aio->cygaud->audio + p_rbuf->rdaddr);
+ regval = readl(aio->audio + p_rbuf->rdaddr);
regval = regval ^ BIT(31);
- writel(regval, aio->cygaud->audio + p_rbuf->wraddr);
+ writel(regval, aio->audio + p_rbuf->wraddr);
} else {
/* Set the ring buffer to empty */
- regval = readl(aio->cygaud->audio + p_rbuf->wraddr);
- writel(regval, aio->cygaud->audio + p_rbuf->rdaddr);
+ regval = readl(aio->audio + p_rbuf->wraddr);
+ writel(regval, aio->audio + p_rbuf->rdaddr);
}
}
@@ -690,7 +690,7 @@ static int cygnus_pcm_prepare(struct snd_pcm_substream
*sub
[PATCH 8/9] ASoC: cygnus: Add EXPORT_SYMBOL for helper function
The helper function cygnus_ssp_set_custom_fsync_width() is intended
to be called from an ASoC machine driver, need to export symbol
if using modules.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index e72d8a8..97bf67a 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -875,6 +875,7 @@ int cygnus_ssp_set_custom_fsync_width(struct snd_soc_dai
*cpu_dai, int len)
return -EINVAL;
}
}
+EXPORT_SYMBOL_GPL(cygnus_ssp_set_custom_fsync_width);
static int cygnus_ssp_set_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
{
--
1.9.1
[PATCH 4/9] ASoC: cygnus: Only enable MCLK pins when in use
The MCLK pins are now only enabled when they are in use. Signed-off-by: Lori Hikichi --- sound/soc/bcm/cygnus-ssp.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c index 00fd4dc..4c476ce 100644 --- a/sound/soc/bcm/cygnus-ssp.c +++ b/sound/soc/bcm/cygnus-ssp.c @@ -41,10 +41,10 @@ /* Begin register offset defines */ #define AUD_MISC_SEROUT_OE_REG_BASE 0x01c #define AUD_MISC_SEROUT_SPDIF_OE 12 -#define AUD_MISC_SEROUT_MCLK_OE 3 -#define AUD_MISC_SEROUT_LRCK_OE 2 -#define AUD_MISC_SEROUT_SCLK_OE 1 -#define AUD_MISC_SEROUT_SDAT_OE 0 +#define AUD_MISC_SEROUT_MCLK_OE3 +#define AUD_MISC_SEROUT_LRCK_OE2 +#define AUD_MISC_SEROUT_SCLK_OE1 +#define AUD_MISC_SEROUT_SDAT_OE0 /* AUD_FMM_BF_CTRL_xxx regs */ #define BF_DST_CFG0_OFFSET 0x100 @@ -684,6 +684,11 @@ static int cygnus_ssp_set_sysclk(struct snd_soc_dai *dai, value |= (sel << I2S_OUT_PLLCLKSEL_SHIFT); writel(value, aio->cygaud->audio + aio->regs.i2s_mclk_cfg); + /* Clear bit for active */ + value = readl(aio->cygaud->audio + AUD_MISC_SEROUT_OE_REG_BASE); + value &= ~BIT(AUD_MISC_SEROUT_MCLK_OE + (aio->portnum * 4)); + writel(value, aio->cygaud->audio + AUD_MISC_SEROUT_OE_REG_BASE); + return 0; } @@ -827,15 +832,12 @@ static int cygnus_ssp_set_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) * Shift the mask based upon port number. */ mask = BIT(AUD_MISC_SEROUT_LRCK_OE) - | BIT(AUD_MISC_SEROUT_SCLK_OE) - | BIT(AUD_MISC_SEROUT_MCLK_OE); + | BIT(AUD_MISC_SEROUT_SCLK_OE); mask = mask << (aio->portnum * 4); if (aio->is_slave) - /* Set bit for tri-state */ - val |= mask; + val |= mask; /* Set bit for tri-state */ else - /* Clear bit for drive */ - val &= ~mask; + val &= ~mask; /* Clear bit for drive */ dev_dbg(aio->cygaud->dev, "%s Set OE bits 0x%x\n", __func__, val); writel(val, aio->cygaud->audio + AUD_MISC_SEROUT_OE_REG_BASE); -- 1.9.1
[PATCH 5/9] ASoC: cygnus: Remove support for 8 bit audio and for mono
These modes of operation were not working properly. There is little to
be gained by enabling these modes and the changes required to
potentially fix these modes would complicate the driver.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 24 +---
1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index 4c476ce..5b6e345 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -547,23 +547,10 @@ static int cygnus_ssp_hw_params(struct snd_pcm_substream
*substream,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
value = readl(aio->cygaud->audio + aio->regs.bf_sourcech_cfg);
value &= ~BIT(BF_SRC_CFGX_BUFFER_PAIR_ENABLE);
- /* Configure channels as mono or stereo/TDM */
- if (params_channels(params) == 1)
- value |= BIT(BF_SRC_CFGX_SAMPLE_CH_MODE);
- else
- value &= ~BIT(BF_SRC_CFGX_SAMPLE_CH_MODE);
+ value &= ~BIT(BF_SRC_CFGX_SAMPLE_CH_MODE);
writel(value, aio->cygaud->audio + aio->regs.bf_sourcech_cfg);
switch (params_format(params)) {
- case SNDRV_PCM_FORMAT_S8:
- if (aio->port_type == PORT_SPDIF) {
- dev_err(aio->cygaud->dev,
- "SPDIF does not support 8bit format\n");
- return -EINVAL;
- }
- bitres = 8;
- break;
-
case SNDRV_PCM_FORMAT_S16_LE:
bitres = 16;
break;
@@ -1008,19 +995,18 @@ static int cygnus_ssp_set_pll(struct snd_soc_dai
*cpu_dai, int pll_id,
#define INIT_CPU_DAI(num) { \
.name = "cygnus-ssp" #num, \
.playback = { \
- .channels_min = 1, \
+ .channels_min = 2, \
.channels_max = 16, \
.rates = SNDRV_PCM_RATE_KNOT, \
- .formats = SNDRV_PCM_FMTBIT_S8 | \
- SNDRV_PCM_FMTBIT_S16_LE | \
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | \
SNDRV_PCM_FMTBIT_S32_LE, \
}, \
.capture = { \
.channels_min = 2, \
.channels_max = 16, \
.rates = SNDRV_PCM_RATE_KNOT, \
- .formats = SNDRV_PCM_FMTBIT_S16_LE | \
- SNDRV_PCM_FMTBIT_S32_LE, \
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | \
+ SNDRV_PCM_FMTBIT_S32_LE, \
}, \
.ops = &cygnus_ssp_dai_ops, \
}
--
1.9.1
[PATCH 7/9] ASoC: cygnus: Remove set_fmt from SPDIF dai ops
The SPDIF port cannot modify its format so a set_fmt function is not
needed. Previously, we used a generic set_fmt for all ports and returned
an error code for the SPDIF port. It is cleaner to not populate the
set_fmt field.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index 5292c04..e72d8a8 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -1222,6 +1222,13 @@ static void update_ssp_cfg(struct cygnus_aio_port *aio)
.set_pll= cygnus_ssp_set_pll,
};
+static const struct snd_soc_dai_ops cygnus_spdif_dai_ops = {
+ .startup= cygnus_ssp_startup,
+ .shutdown = cygnus_ssp_shutdown,
+ .trigger= cygnus_ssp_trigger,
+ .hw_params = cygnus_ssp_hw_params,
+ .set_sysclk = cygnus_ssp_set_sysclk,
+};
#define INIT_CPU_DAI(num) { \
.name = "cygnus-ssp" #num, \
@@ -1255,9 +1262,9 @@ static void update_ssp_cfg(struct cygnus_aio_port *aio)
.channels_max = 2,
.rates = SNDRV_PCM_RATE_KNOT,
.formats = SNDRV_PCM_FMTBIT_S16_LE |
- SNDRV_PCM_FMTBIT_S32_LE,
+ SNDRV_PCM_FMTBIT_S32_LE,
},
- .ops = &cygnus_ssp_dai_ops,
+ .ops = &cygnus_spdif_dai_ops,
};
static struct snd_soc_dai_driver cygnus_ssp_dai[CYGNUS_MAX_PORTS];
--
1.9.1
[PATCH 6/9] ASoc: cygnus: Fix problems with multichannel transfers
Problems were found with multi-channel (4+) TDM transfers. The alignment
of the channels within the frame could shift when starting a new transfer.
In order to implement a fix the register programming sequence needed to
be revised.
Signed-off-by: Lori Hikichi
---
sound/soc/bcm/cygnus-ssp.c | 539 -
sound/soc/bcm/cygnus-ssp.h | 14 +-
2 files changed, 394 insertions(+), 159 deletions(-)
diff --git a/sound/soc/bcm/cygnus-ssp.c b/sound/soc/bcm/cygnus-ssp.c
index 5b6e345..5292c04 100644
--- a/sound/soc/bcm/cygnus-ssp.c
+++ b/sound/soc/bcm/cygnus-ssp.c
@@ -121,6 +121,7 @@
#define I2S_OUT_STREAM_ENA 31
#define I2S_OUT_STREAM_CFG_GROUP_ID 20
#define I2S_OUT_STREAM_CFG_CHANNEL_GROUPING 24
+#define I2S_OUT_STREAM_CFG_FCI_ID_MASK 0x3ff
/* AUD_FMM_IOP_IN_I2S_x_CAP */
#define I2S_IN_STREAM_CFG_CAP_ENA 31
@@ -129,7 +130,11 @@
/* AUD_FMM_IOP_OUT_I2S_x_I2S_CFG_REG */
#define I2S_OUT_CFGX_CLK_ENA 0
#define I2S_OUT_CFGX_DATA_ENABLE 1
+#define I2S_OUT_CFGX_LRCK_POLARITY 4
+#define I2S_OUT_CFGX_SCLK_POLARITY 5
#define I2S_OUT_CFGX_DATA_ALIGNMENT 6
+#define I2S_OUT_CFGX_BITS_PER_SAMPLE 8
+#define I2S_OUT_CFGX_BIT_PER_SAMPLE_MASK 0x1f
#define I2S_OUT_CFGX_BITS_PER_SLOT 13
#define I2S_OUT_CFGX_VALID_SLOT 14
#define I2S_OUT_CFGX_FSYNC_WIDTH18
@@ -137,14 +142,27 @@
#define I2S_OUT_CFGX_SLAVE_MODE 30
#define I2S_OUT_CFGX_TDM_MODE 31
+#define I2S_IN_CFGX_DATA_ALIGNMENT 6
+#define I2S_IN_CFGX_BITS_PER_SAMPLE 8
+#define I2S_IN_CFGX_BIT_PER_SAMPLE_MASK 0x1f
+#define I2S_IN_CFGX_BITS_PER_SLOT 13
+#define I2S_IN_CFGX_VALID_SLOT 14
+#define I2S_IN_CFGX_SLAVE_MODE 30
+#define I2S_IN_CFGX_TDM_MODE31
+
/* AUD_FMM_BF_CTRL_SOURCECH_CFGx_REG */
#define BF_SRC_CFGX_SFIFO_ENA 0
#define BF_SRC_CFGX_BUFFER_PAIR_ENABLE 1
#define BF_SRC_CFGX_SAMPLE_CH_MODE 2
#define BF_SRC_CFGX_SFIFO_SZ_DOUBLE5
#define BF_SRC_CFGX_NOT_PAUSE_WHEN_EMPTY 10
+#define BF_SRC_CFGX_SAMPLE_REPEAT_ENABLE 11
#define BF_SRC_CFGX_BIT_RES 20
#define BF_SRC_CFGX_PROCESS_SEQ_ID_VALID 31
+#define BF_SRC_CFGX_BITRES_MASK 0x1f
+
+/* AUD_FMM_BF_CTRL_SOURCECH_CTRLx_REG */
+#define BF_SOURCECH_CTRL_PLAY_RUN 0
/* AUD_FMM_BF_CTRL_DESTCH_CFGx_REG */
#define BF_DST_CFGX_CAP_ENA 0
@@ -154,11 +172,16 @@
#define BF_DST_CFGX_FCI_ID 12
#define BF_DST_CFGX_CAP_MODE24
#define BF_DST_CFGX_PROC_SEQ_ID_VALID 31
+#define BF_DST_CFGX_BITRES_MASK 0x1f
+
+/* AUD_FMM_BF_CTRL_DESTCH_CTRLX */
+#define BF_DESTCH_CTRLX_CAP_RUN 0x1
/* AUD_FMM_IOP_OUT_SPDIF_xxx */
#define SPDIF_0_OUT_DITHER_ENA 3
#define SPDIF_0_OUT_STREAM_ENA31
+#define IOP_LOGIC_RESET_IN_OFFSET(x) ((x) + 7) /* Capture ports offset by 7 */
#define INIT_SSP_REGS(num) (struct cygnus_ssp_regs){ \
.i2s_stream_cfg = OUT_I2S_ ##num## _STREAM_CFG_OFFSET, \
@@ -169,8 +192,7 @@
.bf_destch_ctrl = BF_DST_CTRL ##num## _OFFSET, \
.bf_destch_cfg = BF_DST_CFG ##num## _OFFSET, \
.bf_sourcech_ctrl = BF_SRC_CTRL ##num## _OFFSET, \
- .bf_sourcech_cfg = BF_SRC_CFG ##num## _OFFSET, \
- .bf_sourcech_grp = BF_SRC_GRP ##num## _OFFSET \
+ .bf_sourcech_cfg = BF_SRC_CFG ##num## _OFFSET \
}
#define CYGNUS_RATE_MIN 8000
@@ -189,6 +211,8 @@
.list = cygnus_rates,
};
+static void update_ssp_cfg(struct cygnus_aio_port *aio);
+
static struct cygnus_aio_port *cygnus_dai_get_portinfo(struct snd_soc_dai *dai)
{
struct cygnus_audio *cygaud = snd_soc_dai_get_drvdata(dai);
@@ -201,15 +225,17 @@ static int audio_ssp_init_portregs(struct cygnus_aio_port
*aio)
u32 value, fci_id;
int status = 0;
+ /* Set Group ID */
+ writel(0, aio->cygaud->audio + BF_SRC_GRP0_OFFSET);
+ writel(1, aio->cygaud->audio + BF_SRC_GRP1_OFFSET);
+ writel(2, aio->cygaud->audio + BF_SRC_GRP2_OFFSET);
+ writel(3, aio->cygaud->audio + BF_SRC_GRP3_OFFSET);
+
switch (aio->port_type) {
case PORT_TDM:
value = readl(aio->cygaud->audio + aio->regs.i2s_stream_cfg);
value &= ~I2S_STREAM_CFG_MASK;
- /* Set Group ID */
- writel(aio->portnum,
- aio->cygaud->audio + aio->regs.bf_sourcech_grp);
-
/* Configure the AUD_FMM_IOP_OUT_I2S_x_STREAM_CFG reg */
value |= aio->portnum << I2S_OUT_STREAM_CFG_GROUP_ID;
value |= aio->portnum; /* FCI ID is the port num */
@@ -219,6 +245,7 @@ static int audio_ssp_init_portregs(struct cygnus_aio_port
*aio)
/* Configure the AUD_FMM_BF_CTRL_SOURCECH_CFGX reg */
value = readl(aio->cygaud->audio + aio->regs.bf_sourcech_cfg);
value &= ~BIT(BF_SRC_CFGX_NOT_PAUS
[PATCH 0/9] ASoC: cygnus: Various improvements and fixes
This patch series contains an number of improvements and refinements to the driver. There is also a fix for a problem when transferring four or more channels in TDM mode. Lori Hikichi (9): ASoC: cygnus: Add support for 384kHz frame rates ASoC: cygnus: Update bindings for audio clock changes ASoC: cygnus: Allow each port to select its clock source ASoC: cygnus: Only enable MCLK pins when in use ASoC: cygnus: Remove support for 8 bit audio and for mono ASoc: cygnus: Fix problems with multichannel transfers ASoC: cygnus: Remove set_fmt from SPDIF dai ops ASoC: cygnus: Add EXPORT_SYMBOL for helper function ASoC: cygnus: Tidy up of structure access .../bindings/sound/brcm,cygnus-audio.txt | 70 +- sound/soc/bcm/cygnus-pcm.c | 50 +- sound/soc/bcm/cygnus-ssp.c | 1336 ++-- sound/soc/bcm/cygnus-ssp.h | 51 +- 4 files changed, 739 insertions(+), 768 deletions(-) -- 1.9.1
[PATCH 2/9] ASoC: cygnus: Update bindings for audio clock changes
Allow each audio port to select which clock (if any) it wants to use.
Signed-off-by: Lori Hikichi
---
.../bindings/sound/brcm,cygnus-audio.txt | 70 ++
1 file changed, 45 insertions(+), 25 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt
b/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt
index b139e66..2ef2f2c 100644
--- a/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt
+++ b/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt
@@ -9,19 +9,28 @@ Required properties:
Valid names are "aud" and "i2s_in". "aud" contains a
set of DMA, I2S_OUT and SPDIF registers. "i2s_in" contains
a set of I2S_IN registers.
- - clocks: PLL and leaf clocks used by audio ports
- - assigned-clocks: PLL and leaf clocks
- - assigned-clock-parents: parent clocks of the assigned clocks
- (usually the PLL)
- - assigned-clock-rates: List of clock frequencies of the
- assigned clocks
- - clock-names: names of 3 leaf clocks used by audio ports
- Valid names are "ch0_audio", "ch1_audio", "ch2_audio"
- interrupts: audio DMA interrupt number
+Optional properties:
+ - assigned-clocks: only valid choice is audiopll
+ - assigned-clock-rates: clock frequency for audiopll
+If none of the ports need an internal master clock then there no need to
+initialize the pll clock.
+
+
SSP Subnode properties:
-- reg: The index of ssp port interface to use
- Valid value are 0, 1, 2, or 3 (for spdif)
+Required:
+ - reg: The index of ssp port interface to use
+ Valid value are 0, 1, 2, or 3 (for spdif)
+Optional:
+ - clocks: clock used by audio port
+ one of the audiopll outputs (see brcm,iproc-clocks.txt).
+ - clock-names: Must be "ssp_clk"
+ - brcm,ssp-clk-mux = Needed if a clock is named and used. This value is
+ used to program the mux within the audio driver which
selects
+ the incoming clock. Here is the mapping.
+ audio_pll output 0 = 0, output 1 = 1, and output 2 = 2
+
Example:
cygnus_audio: audio@180ae000 {
@@ -30,38 +39,49 @@ Example:
#size-cells = <0>;
reg = <0x180ae000 0xafd>, <0x180aec00 0x1f8>;
reg-names = "aud", "i2s_in";
- clocks = <&audiopll BCM_CYGNUS_AUDIOPLL_CH0>,
- <&audiopll BCM_CYGNUS_AUDIOPLL_CH1>,
- <&audiopll BCM_CYGNUS_AUDIOPLL_CH2>;
- assigned-clocks = <&audiopll BCM_CYGNUS_AUDIOPLL>,
- <&audiopll
BCM_CYGNUS_AUDIOPLL_CH0>,
- <&audiopll
BCM_CYGNUS_AUDIOPLL_CH1>,
- <&audiopll
BCM_CYGNUS_AUDIOPLL_CH2>;
- assigned-clock-parents = <&audiopll BCM_CYGNUS_AUDIOPLL>;
- assigned-clock-rates = <1769470191>,
- <0>,
- <0>,
- <0>;
- clock-names = "ch0_audio", "ch1_audio", "ch2_audio";
+
+ assigned-clocks = <&audiopll BCM_CYGNUS_AUDIOPLL>;
+ assigned-clock-rates = <1376255989>;
+
interrupts = ;
ssp0: ssp_port@0 {
reg = <0>;
+
+ clocks = <&audiopll BCM_CYGNUS_AUDIOPLL_CH0>;
+ clock-names = "ssp_clk";
+ brcm,ssp-clk-mux = <0>;
+
status = "okay";
};
ssp1: ssp_port@1 {
reg = <1>;
- status = "disabled";
+
+ clocks = <&audiopll BCM_CYGNUS_AUDIOPLL_CH1>;
+ clock-names = "ssp_clk";
+ brcm,ssp-clk-mux = <1>;
+
+ status = "okay";
};
ssp2: ssp_port@2 {
reg = <2>;
- status = "disabled";
+
+ clocks = <&audiopll BCM_CYGNUS_AUDIOPLL_CH2>;
+ clock-names = "ssp_clk";
+ brcm,ssp-clk-mux = <2>;
+
+ status = "okay";
};
spdif: spdif_port@3 {
reg = <3>;
+
+ clocks = <&audiopll BCM_CYGNUS_AUDIOPLL_CH2>;
+ clock-names = "ssp_clk";
+ brcm,ssp-clk-mux = <2>;
+
status = "disabled";
};
};
--
1.9.1
[PATCH v1 2/4] clk: iproc: Fix error in the pll post divider rate calculation
The pll post divider code was using DIV_ROUND_UP when determining the
divider value best suited to produce the target frequency.
Using DIV_ROUND_CLOSEST will give us better divider values when
the division results in a small remainder.
Also, change the post divider clock over to the determine_rate api
instead of round_rate.
Signed-off-by: Simran Rai
Signed-off-by: Lori Hikichi
---
drivers/clk/bcm/clk-iproc-pll.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 9514ecf..7df010b 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -602,25 +602,26 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw
*hw,
return clk->rate;
}
-static long iproc_clk_round_rate(struct clk_hw *hw, unsigned long rate,
- unsigned long *parent_rate)
+static int iproc_clk_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
{
- unsigned int div;
+ unsigned int bestdiv;
- if (rate == 0 || *parent_rate == 0)
+ if (req->rate == 0)
return -EINVAL;
+ if (req->rate == req->best_parent_rate)
+ return 0;
- if (rate == *parent_rate)
- return *parent_rate;
+ bestdiv = DIV_ROUND_CLOSEST(req->best_parent_rate, req->rate);
+ if (bestdiv < 2)
+ req->rate = req->best_parent_rate;
- div = DIV_ROUND_UP(*parent_rate, rate);
- if (div < 2)
- return *parent_rate;
+ if (bestdiv > 256)
+ bestdiv = 256;
- if (div > 256)
- div = 256;
+ req->rate = req->best_parent_rate / bestdiv;
- return *parent_rate / div;
+ return 0;
}
static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -635,10 +636,10 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned
long rate,
if (rate == 0 || parent_rate == 0)
return -EINVAL;
+ div = DIV_ROUND_CLOSEST(parent_rate, rate);
if (ctrl->flags & IPROC_CLK_MCLK_DIV_BY_2)
- div = DIV_ROUND_UP(parent_rate, rate * 2);
- else
- div = DIV_ROUND_UP(parent_rate, rate);
+ div /= 2;
+
if (div > 256)
return -EINVAL;
@@ -662,7 +663,7 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned
long rate,
.enable = iproc_clk_enable,
.disable = iproc_clk_disable,
.recalc_rate = iproc_clk_recalc_rate,
- .round_rate = iproc_clk_round_rate,
+ .determine_rate = iproc_clk_determine_rate,
.set_rate = iproc_clk_set_rate,
};
--
1.9.1
[PATCH v1 3/4] clk: iproc: Allow plls to do minor rate changes without reset
From: Lori Hikichi
The iproc plls are capable of doing small rate changes without the
need for a full reset and re-lock procedure. This feature will
allow for small tweaks to the PLL rate to occur smoothly.
Signed-off-by: Lori Hikichi
---
drivers/clk/bcm/clk-iproc-pll.c | 47 +
1 file changed, 47 insertions(+)
diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 7df010b..ab10819 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -285,6 +285,40 @@ static void __pll_bring_out_reset(struct iproc_pll *pll,
unsigned int kp,
iproc_pll_write(pll, pll->control_base, reset->offset, val);
}
+/*
+ * Determines if the change to be applied to the PLL is minor (just an update
+ * or the fractional divider). If so, then we can avoid going through a
+ * disruptive reset and lock sequence.
+ */
+static bool pll_fractional_change_only(struct iproc_pll *pll,
+ struct iproc_pll_vco_param *vco)
+{
+ const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+ u32 val;
+ u32 ndiv_int;
+ unsigned int pdiv;
+
+ /* PLL needs to be locked */
+ val = readl(pll->status_base + ctrl->status.offset);
+ if ((val & (1 << ctrl->status.shift)) == 0)
+ return false;
+
+ val = readl(pll->control_base + ctrl->ndiv_int.offset);
+ ndiv_int = (val >> ctrl->ndiv_int.shift) &
+ bit_mask(ctrl->ndiv_int.width);
+
+ if (ndiv_int != vco->ndiv_int)
+ return false;
+
+ val = readl(pll->control_base + ctrl->pdiv.offset);
+ pdiv = (val >> ctrl->pdiv.shift) & bit_mask(ctrl->pdiv.width);
+
+ if (pdiv != vco->pdiv)
+ return false;
+
+ return true;
+}
+
static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
unsigned long parent_rate)
{
@@ -333,6 +367,19 @@ static int pll_set_rate(struct iproc_clk *clk, struct
iproc_pll_vco_param *vco,
return ret;
}
+ if (pll_fractional_change_only(clk->pll, vco)) {
+ /* program fractional part of NDIV */
+ if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
+ val = readl(pll->control_base + ctrl->ndiv_frac.offset);
+ val &= ~(bit_mask(ctrl->ndiv_frac.width) <<
+ctrl->ndiv_frac.shift);
+ val |= vco->ndiv_frac << ctrl->ndiv_frac.shift;
+ iproc_pll_write(pll, pll->control_base,
+ ctrl->ndiv_frac.offset, val);
+ return 0;
+ }
+ }
+
/* put PLL in reset */
__pll_put_in_reset(pll);
--
1.9.1
[PATCH v1 4/4] clk: iproc: Minor tidy up of iproc pll data structures
From: Lori Hikichi
There were a few fields in the iproc pll data structures that were
holding information that was not true state information.
Using stack variables is sufficient and simplifies the structure.
There are not any functional changes in this commit.
Signed-off-by: Lori Hikichi
---
drivers/clk/bcm/clk-iproc-pll.c | 83 ++---
1 file changed, 36 insertions(+), 47 deletions(-)
diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index ab10819..43a58ae 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -69,16 +69,6 @@ enum vco_freq_range {
VCO_MAX = 40U,
};
-struct iproc_pll;
-
-struct iproc_clk {
- struct clk_hw hw;
- const char *name;
- struct iproc_pll *pll;
- unsigned long rate;
- const struct iproc_clk_ctrl *ctrl;
-};
-
struct iproc_pll {
void __iomem *status_base;
void __iomem *control_base;
@@ -88,9 +78,12 @@ struct iproc_pll {
const struct iproc_pll_ctrl *ctrl;
const struct iproc_pll_vco_param *vco_param;
unsigned int num_vco_entries;
+};
- struct clk_hw_onecell_data *clk_data;
- struct iproc_clk *clks;
+struct iproc_clk {
+ struct clk_hw hw;
+ struct iproc_pll *pll;
+ const struct iproc_clk_ctrl *ctrl;
};
#define to_iproc_clk(hw) container_of(hw, struct iproc_clk, hw)
@@ -329,6 +322,7 @@ static int pll_set_rate(struct iproc_clk *clk, struct
iproc_pll_vco_param *vco,
u32 val;
enum kp_band kp_index;
unsigned long ref_freq;
+ const char *clk_name = clk_hw_get_name(&clk->hw);
/*
* reference frequency = parent frequency / PDIV
@@ -351,19 +345,19 @@ static int pll_set_rate(struct iproc_clk *clk, struct
iproc_pll_vco_param *vco,
kp_index = KP_BAND_HIGH_HIGH;
} else {
pr_err("%s: pll: %s has invalid rate: %lu\n", __func__,
- clk->name, rate);
+ clk_name, rate);
return -EINVAL;
}
kp = get_kp(ref_freq, kp_index);
if (kp < 0) {
- pr_err("%s: pll: %s has invalid kp\n", __func__, clk->name);
+ pr_err("%s: pll: %s has invalid kp\n", __func__, clk_name);
return kp;
}
ret = __pll_enable(pll);
if (ret) {
- pr_err("%s: pll: %s fails to enable\n", __func__, clk->name);
+ pr_err("%s: pll: %s fails to enable\n", __func__, clk_name);
return ret;
}
@@ -433,7 +427,7 @@ static int pll_set_rate(struct iproc_clk *clk, struct
iproc_pll_vco_param *vco,
ret = pll_wait_for_lock(pll);
if (ret < 0) {
- pr_err("%s: pll: %s failed to lock\n", __func__, clk->name);
+ pr_err("%s: pll: %s failed to lock\n", __func__, clk_name);
return ret;
}
@@ -469,16 +463,15 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw
*hw,
u32 val;
u64 ndiv, ndiv_int, ndiv_frac;
unsigned int pdiv;
+ unsigned long rate;
if (parent_rate == 0)
return 0;
/* PLL needs to be locked */
val = readl(pll->status_base + ctrl->status.offset);
- if ((val & (1 << ctrl->status.shift)) == 0) {
- clk->rate = 0;
+ if ((val & (1 << ctrl->status.shift)) == 0)
return 0;
- }
/*
* PLL output frequency =
@@ -500,14 +493,14 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw
*hw,
val = readl(pll->control_base + ctrl->pdiv.offset);
pdiv = (val >> ctrl->pdiv.shift) & bit_mask(ctrl->pdiv.width);
- clk->rate = (ndiv * parent_rate) >> 20;
+ rate = (ndiv * parent_rate) >> 20;
if (pdiv == 0)
- clk->rate *= 2;
+ rate *= 2;
else
- clk->rate /= pdiv;
+ rate /= pdiv;
- return clk->rate;
+ return rate;
}
static int iproc_pll_determine_rate(struct clk_hw *hw,
@@ -632,6 +625,7 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw
*hw,
struct iproc_pll *pll = clk->pll;
u32 val;
unsigned int mdiv;
+ unsigned long rate;
if (parent_rate == 0)
return 0;
@@ -642,11 +636,11 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw
*hw,
mdiv = 256;
if (ctrl->flags & IPROC_CLK_MCLK_DIV_BY_2)
- clk->rate = parent_rate / (mdiv * 2);
+ rate = parent_rate / (mdiv * 2);
else
- clk->rate = parent_rate / mdiv;
+ rate = parent_rate / mdiv;
- return clk->rate;
+
[PATCH v1 1/4] clk: iproc: Allow iproc pll to runtime calculate vco parameters
Add the ability for the iproc pll to calculate the pll parameters at
runtime instead of only using predefined tables. This ability allows
the clock users to select from the full range of vco frequencies.
The old method of table based programming is retained so that existing
users will retain expected behavior. The flag IPROC_CLK_PLL_CALC_PARAM
will need to be set to enable the new runtime calculation method.
Currently, this is only being enabled for the audio pll.
This feature also revealed a problem with the driver using the
round_rate api. The round_rate api does not allow for frequencies larger
than 2^31 to be returned. Those large frequencies are interpreted as an
error code. Therefore, we are moving to the determine_rate api which
solves this problem.
Signed-off-by: Simran Rai
Signed-off-by: Lori Hikichi
---
drivers/clk/bcm/clk-cygnus.c| 25 +++
drivers/clk/bcm/clk-iproc-pll.c | 97 ++---
drivers/clk/bcm/clk-iproc.h | 5 +++
3 files changed, 92 insertions(+), 35 deletions(-)
diff --git a/drivers/clk/bcm/clk-cygnus.c b/drivers/clk/bcm/clk-cygnus.c
index 464fdc4..b8d073e 100644
--- a/drivers/clk/bcm/clk-cygnus.c
+++ b/drivers/clk/bcm/clk-cygnus.c
@@ -269,23 +269,10 @@ static void __init cygnus_asiu_init(struct device_node
*node)
}
CLK_OF_DECLARE(cygnus_asiu_clk, "brcm,cygnus-asiu-clk", cygnus_asiu_init);
-/*
- * AUDIO PLL VCO frequency parameter table
- *
- * PLL output frequency = ((ndiv_int + ndiv_frac / 2^20) *
- * (parent clock rate / pdiv)
- *
- * On Cygnus, parent is the 25MHz oscillator
- */
-static const struct iproc_pll_vco_param audiopll_vco_params[] = {
- /* rate (Hz) ndiv_int ndiv_frac pdiv */
- { 1354750204UL, 54, 199238, 1 },
- { 1769470191UL, 70, 816639, 1 },
-};
-
static const struct iproc_pll_ctrl audiopll = {
.flags = IPROC_CLK_PLL_NEEDS_SW_CFG | IPROC_CLK_PLL_HAS_NDIV_FRAC |
- IPROC_CLK_PLL_USER_MODE_ON | IPROC_CLK_PLL_RESET_ACTIVE_LOW,
+ IPROC_CLK_PLL_USER_MODE_ON | IPROC_CLK_PLL_RESET_ACTIVE_LOW |
+ IPROC_CLK_PLL_CALC_PARAM,
.reset = RESET_VAL(0x5c, 0, 1),
.dig_filter = DF_VAL(0x48, 0, 3, 6, 4, 3, 3),
.sw_ctrl = SW_CTRL_VAL(0x4, 0),
@@ -300,8 +287,7 @@ static void __init cygnus_asiu_init(struct device_node
*node)
static const struct iproc_clk_ctrl audiopll_clk[] = {
[BCM_CYGNUS_AUDIOPLL_CH0] = {
.channel = BCM_CYGNUS_AUDIOPLL_CH0,
- .flags = IPROC_CLK_AON |
- IPROC_CLK_MCLK_DIV_BY_2,
+ .flags = IPROC_CLK_AON | IPROC_CLK_MCLK_DIV_BY_2,
.enable = ENABLE_VAL(0x14, 8, 10, 9),
.mdiv = REG_VAL(0x14, 0, 8),
},
@@ -321,9 +307,8 @@ static void __init cygnus_asiu_init(struct device_node
*node)
static void __init cygnus_audiopll_clk_init(struct device_node *node)
{
- iproc_pll_clk_setup(node, &audiopll, audiopll_vco_params,
- ARRAY_SIZE(audiopll_vco_params), audiopll_clk,
- ARRAY_SIZE(audiopll_clk));
+ iproc_pll_clk_setup(node, &audiopll, NULL, 0,
+ audiopll_clk, ARRAY_SIZE(audiopll_clk));
}
CLK_OF_DECLARE(cygnus_audiopll, "brcm,cygnus-audiopll",
cygnus_audiopll_clk_init);
diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 375d8dd..9514ecf 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -95,6 +95,39 @@ struct iproc_pll {
#define to_iproc_clk(hw) container_of(hw, struct iproc_clk, hw)
+static int pll_calc_param(unsigned long target_rate,
+ unsigned long parent_rate,
+ struct iproc_pll_vco_param *vco_out)
+{
+ u64 ndiv_int, ndiv_frac, residual;
+
+ ndiv_int = target_rate / parent_rate;
+
+ if (!ndiv_int || (ndiv_int > 255))
+ return -EINVAL;
+
+ residual = target_rate - (ndiv_int * parent_rate);
+ residual <<= 20;
+
+ /*
+* Add half of the divisor so the result will be rounded to closest
+* instead of rounded down.
+*/
+ residual += (parent_rate / 2);
+ ndiv_frac = div64_u64((u64)residual, (u64)parent_rate);
+
+ vco_out->ndiv_int = ndiv_int;
+ vco_out->ndiv_frac = ndiv_frac;
+ vco_out->pdiv = 1;
+
+ vco_out->rate = vco_out->ndiv_int * parent_rate;
+ residual = (u64)vco_out->ndiv_frac * (u64)parent_rate;
+ residual >>= 20;
+ vco_out->rate += residual;
+
+ return 0;
+}
+
/*
* Based on the target frequency, find a match from the VCO frequency parameter
* table and return its index
@@ -252,11 +285,10 @@ static void __pll_bring_out_reset(struct iproc_pll *pll,
unsigned int kp,
iproc_pll_write(pll, pll->control_base, reset->offset, val);
}
-static int pl
[PATCH v1 0/4] clk: iproc: Enable glitchless pll rate change
This patchset enables the ability for the iproc plls to do small rate changes without glitching the clock. Lori Hikichi (4): clk: iproc: Allow iproc pll to runtime calculate vco parameters clk: iproc: Fix error in the pll post divider rate calculation clk: iproc: Allow plls to do minor rate changes without reset clk: iproc: Minor tidy up of iproc pll data structures drivers/clk/bcm/clk-cygnus.c| 25 +--- drivers/clk/bcm/clk-iproc-pll.c | 260 drivers/clk/bcm/clk-iproc.h | 5 + 3 files changed, 192 insertions(+), 98 deletions(-) -- 1.9.1
Re: [PATCH 0/2] Cygnus Audio Driver
On 15-04-08 11:54 AM, Mark Brown wrote: > On Tue, Apr 07, 2015 at 07:28:40PM -0700, Lori Hikichi wrote: >> On 15-04-06 02:58 AM, Mark Brown wrote: > >>> OK, then it's going to need to be a clock provider at some point - the >>> clock will be going into external devices which are going to need to be >>> able to interact with the clock (for example, to get the rate). > >> Currently, the ASoC machine driver is responsible for requesting a certain >> frequency of MCLK be generated from our driver and then also sending the >> frequency information along to the external device (codec). >> This is done via the snd_soc_dai_set_sysclk. That is the only clock >> interaction we have needed for the core part of the driver. For enhanced > > I have some passing familiarity with ASoC... if you look at newer > drivers, especially those for DT systems, you'll see that we're > transitioning CODEC drivers to use the clock API for their clocks since > this makes integrating with both generic ASoC things like simple card > and non-ASoC clocks. > >> features, we also have the need to make minor adjustments (tweaks) to the >> PLL. The tweaks are used to make the PLLs output frequency match as closely >> as possible to a true reference frequency. As such, we would like to provide >> the finest adjustment resolution as possible. The clocking framework only >> seems to allow for a 1 Hz adjustment. This limitation and the fact that no >> other device seems to need to interact directly will the PLL are why we have >> not put it in the clocking framework. > > That's going to be an issue no matter where you put the control - the > ASoC specific clocking APIs don't have any control here either. I don't > know if we want to add the functionality for doing very fine grained > adjustments into the clock API or not (the use cases seem limited though > I'm sure they exist), though I do think we should have that discussion > if only to confirm, but that's a separate thing to how we expose any > userspace control - the clock API is a kernel internal thing. > Seems like there are some benefits to integrating with the clocking framework. I will have to consider what kernel APIs we want to expose as well. I believe we may have another kernel driver wanting to control this tweaking. Do you feel it is ok to have to PLL code reside in this driver for now, and then we can patch later after we get this clocking control sorted out. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ASoC: add core audio driver for Broadcom Cygnus SOC.
On 15-04-06 09:19 AM, Mark Brown wrote: On Thu, Apr 02, 2015 at 11:47:18AM -0700, Lori Hikichi wrote: On 15-03-30 11:42 PM, Mark Brown wrote: +config SND_SOC_CYGNUS + tristate "SoC platform audio for Broadcom Cygnus chips" + depends on ARCH_BCM_CYGNUS || COMPILE_TEST + default ARCH_BCM_CYGNUS Okay. You don't need to reply to every single comment, the general assumption will be that if there's no other followup all review comments will be addressed. It's better to just reply to things where there's something more detailed to say, if you explicitly reply to everything then that makes it easier for actual replies to be missed since there's a lot of there's a lot of the mail that's just going to be skipped through. +static void ringbuf_inc(void __iomem *audio_io, bool is_playback, + const struct ringbuf_regs *p_rbuf) So it looks like we're getting an interrupt per period and we have to manually advance to the next one? Yes. OK, so that seems fragile - what happens if we're slightly late processing an interrupt or miss one entirely? Most hardware has some way to read back the current position which tends to be more reliable, is that not an option here? The hardware updates a read pointer (rdaddr) which we feed to ALSA via the ".pointer" hook. So yes, the hardware does have a register that tells us its progress. This routine (ringbuf_inc) actually updates a write pointer (wraddr) which is a misnomer. The write pointer doesn’t actually tell us where we are writing to – ALSA keeps track of that. The wraddr only prevents the hardware from reading past it. We just use it, along with a low water mark configuration register, to keep the periodic interrupts firing. The hardware is tracking the distance between rdaddr and wraddr and comparing that to the low water mark. Being late processing the interrupt is okay since there are more samples to read. That is, it was only a low water mark interrupt and we have one period of valid data still (we configure low water to be one period). Missing an interrupt is okay since the hardware will just stop reading from the ring buffer when rdaddr has hit wraddr. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] Cygnus Audio Driver
On 15-04-06 02:58 AM, Mark Brown wrote: On Fri, Apr 03, 2015 at 12:33:12PM -0700, Scott Branden wrote: On 15-03-30 11:43 PM, Mark Brown wrote: On Mon, Mar 30, 2015 at 08:16:22PM -0700, Scott Branden wrote: The audio PLL is embedded in the audio block and only used by the audio block. The audio PLL registers are also in the middle of the audio register map. When you say it's only used by the audio block do you mean to say that the audio block exposes no clock signals other than the bit and frame clocks? The audio block exposes the MCLK in addition to the bit and frame clock. OK, then it's going to need to be a clock provider at some point - the clock will be going into external devices which are going to need to be able to interact with the clock (for example, to get the rate). Currently, the ASoC machine driver is responsible for requesting a certain frequency of MCLK be generated from our driver and then also sending the frequency information along to the external device (codec). This is done via the snd_soc_dai_set_sysclk. That is the only clock interaction we have needed for the core part of the driver. For enhanced features, we also have the need to make minor adjustments (tweaks) to the PLL. The tweaks are used to make the PLLs output frequency match as closely as possible to a true reference frequency. As such, we would like to provide the finest adjustment resolution as possible. The clocking framework only seems to allow for a 1 Hz adjustment. This limitation and the fact that no other device seems to need to interact directly will the PLL are why we have not put it in the clocking framework. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCH 1/2] ASoC: cygnus-audio: adding device tree bindings
On 15-03-31 12:26 AM, Lars-Peter Clausen wrote: On 03/31/2015 05:16 AM, Scott Branden wrote: [...] +- ssp-port-id: The ssp port interface to use +Valid value are 0, 1, 2, or 3 (for spdif) How about using 'reg' as the property name here. It is the standard property name for identifying or assigning an address to a sub-node. Okay. Thanks. I will change. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ASoC: add core audio driver for Broadcom Cygnus SOC.
On 15-03-30 11:42 PM, Mark Brown wrote:
On Mon, Mar 30, 2015 at 08:16:24PM -0700, Scott Branden wrote:
The CC list for this patch is pretty wide - please look at who you're
sending this to and try to send to only relevant people (for example I'm
not sure the Raspberry Pi people need to review this). People working
upstream get a lot of mail so it's helpful to avoid filling their
inboxes with random irrelevant stuff.
sound/soc/bcm/Kconfig | 11 +
sound/soc/bcm/Makefile |5 +-
sound/soc/bcm/cygnus-pcm.c | 918 +
sound/soc/bcm/cygnus-pcm.h | 45 ++
sound/soc/bcm/cygnus-ssp.c | 1613
sound/soc/bcm/cygnus-ssp.h | 84 +++
6 files changed, 2675 insertions(+), 1 deletion(-)
Send the DMA and DAI drivers as separate patches please, it makes review
easier.
+config SND_SOC_CYGNUS
+ tristate "SoC platform audio for Broadcom Cygnus chips"
+ depends on ARCH_BCM_CYGNUS || COMPILE_TEST
+ default ARCH_BCM_CYGNUS
Okay.
Remove the default setting here - we don't do this for other drivers, we
shouldn't do it here.
+/*
+ * PERIOD_BYTES_MIN is the number of bytes to at which the interrupt will tick.
+ * This number should be a multiple of 256
+ */
+#define PERIOD_BYTES_MIN 0x100
This sounds like it's a setting rather than actually the minimum?
It is a bad comment. I will update. This is the minimum period (in
bytes) at which the interrupt can tick. Other possible value for the
period must be multiple of this value.
+static const struct snd_pcm_hardware cygnus_pcm_hw = {
+ .info = SNDRV_PCM_INFO_MMAP |
+ SNDRV_PCM_INFO_MMAP_VALID |
+ SNDRV_PCM_INFO_INTERLEAVED,
The DMA controller is integrated into the IP?
Yes, it is dedicated for the audio driver.
+static int enable_count;
This looks bogus - why is this a global variable not part of the device
struct and if it does need to be global why does it need no locking?
I will fix.
+ if (aio->portnum == 0)
+ *p_rbuf = RINGBUF_REG_PLAYBACK(0);
+ else if (aio->portnum == 1)
+ *p_rbuf = RINGBUF_REG_PLAYBACK(2);
+ else if (aio->portnum == 2)
+ *p_rbuf = RINGBUF_REG_PLAYBACK(4);
+ else if (aio->portnum == 3)
+ *p_rbuf = RINGBUF_REG_PLAYBACK(6); /*SPDIF */
+ else
+ status = -EINVAL;
This looks like a switch statement, there are many places in the code
where you're writing switch statements as chains of ifs.
No problem. Will use switch statements.
+static void ringbuf_inc(void __iomem *audio_io, bool is_playback,
+ const struct ringbuf_regs *p_rbuf)
+{
+ u32 regval, endval, active_ptr;
+
+ if (is_playback)
+ active_ptr = p_rbuf->wraddr;
+ else
+ active_ptr = p_rbuf->rdaddr;
+
+ endval = readl(audio_io + p_rbuf->endaddr);
+ regval = readl(audio_io + active_ptr);
+ regval = regval + p_rbuf->period_bytes;
+ if (regval > endval)
+ regval -= p_rbuf->buf_size;
+
+ writel(regval, audio_io + active_ptr);
+}
So it looks like we're getting an interrupt per period and we have to
manually advance to the next one?
Yes.
+static irqreturn_t cygnus_dma_irq(int irq, void *data)
+{
+ u32 r5_status;
+ struct cygnus_audio *cygaud;
+
+ cygaud = (struct cygnus_audio *)data;
If you need to cast away from void something is very wrong.
Okay, will fix.
+ /*
+* R5 status bits Description
+* 0 ESR0 (playback FIFO interrupt)
+* 1 ESR1 (playback rbuf interrupt)
+* 2 ESR2 (capture rbuf interrupt)
+* 3 ESR3 (Freemark play. interrupt)
+* 4 ESR4 (Fullmark capt. interrupt)
+*/
+ r5_status = readl(cygaud->audio + INTH_R5F_STATUS_OFFSET);
+
+ /* If playback interrupt happened */
+ if (ANY_PLAYBACK_IRQ & r5_status)
+ handle_playback_irq(cygaud);
+
+ /* If capture interrupt happened */
+ if (ANY_CAPTURE_IRQ & r5_status)
+ handle_capture_irq(cygaud);
+
+ /*
+* clear r5 interrupts after servicing them to avoid overwriting
+* esr_status
+*/
+ writel(r5_status, cygaud->audio + INTH_R5F_CLEAR_OFFSET);
This feels racy especially given that we seem to need every interrupt
delivering. What if another period happens during the servicing? I
don't understand what "overwriting esr_status" means here.
Let me review this handler and I will enhance as needed.
+ return IRQ_HANDLED;
If neither playback nor capture interrupts were flagged we should return
IRQ_NONE.
Okay, will fix.
+/*
+ * This code is identical to what is done by the framework, when we do not
+ * supply a 'copy' function. Ha
Re: [PATCH 1/2] ASoC: cygnus-audio: adding device tree bindings
On 15-03-30 10:58 PM, Mark Brown wrote: On Mon, Mar 30, 2015 at 08:16:23PM -0700, Scott Branden wrote: +SSP Subnode properties: +- dai-name: The name of the DAI registered with ASOC ASoC. Okay. Why is this in the DT - it sounds like this is just an internal implementation detail for Linux, not a property of the hardware. Will move into the driver code. +- mode: Controls if this port should be configured as I2S or TDM mode. + Valid values are: "tdm" or "i2s" +- tdm-bits-per-frame: only if mode is "tdm" then this property must set. + Valid values are (128/256/512) We'd normally leave these up to the machine driver to set as they're link wide things for system integration. The bits per frame in particular looks like something that's not going to be fixed by the hardware and could be varied at runtime. I was using the device tree to set some board specific properties of our audio serial port driver. The idea was that these properties would only need to be one value for a specific board. Therefore, we could set them in the device tree and then not have to set them in the machine file. I thought it would help keep the machine file a little more basic. Moving these to the machine file is reasonable and will allow for a more dynamic usage of our driver. I will move them to machine file. +- port-status: Controls if port is enabled or not + Valid values "enabled" or "disabled" This sounds like it's replicating the DT standard status property? Okay. Will replace with "status" +- channel-group: Control grouping of serial port + Valid values are "2_0", "3_1", or "5_1" What does this mean? It looks like it's setting the number of channels which again seems like a runtime thing. It is a configuration where we can link together two or three of the ports to allow for synchronized starts and stop. I will look into another method of configuring this operational mode. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
