This series is a preparatory cleanup of the jz4740-i2s driver before
adding support for a new SoC. The last two patches lift unnecessary
restrictions on sample rates and formats -- the existing ones appear
to be derived from the limitations of the JZ4740's internal codec and
don't reflect the actual capabilities of the I2S controller.
I'm unable to test the series on any JZ47xx SoCs, but I have tested
on an X1000 (which is the SoC I'll be adding in a followup series).
Changes in v2:
* Drop two patches already in sound for-next.
* Squash two removal patches into the regmap fields patch.
* Remove the unused 'mem' resource in the driver private struct.
* Use regmap_set_bits() and regmap_clear_bits() to improve readability.
* Add fix for SoCs with independent FIFO flush bits (ie. most of them).
* Update sample formats patch with a more informative commit message.
* Add two new patches to refactor DAI/component probing.
Changes in v3:
* Fix missing 'ret' in patch 11 (yes, that was pretty silly of me)
Aidan MacDonald (11):
ASoC: jz4740-i2s: Remove unused 'mem' resource
ASoC: jz4740-i2s: Convert to regmap API
ASoC: jz4740-i2s: Simplify using regmap fields
ASoC: jz4740-i2s: Handle independent FIFO flush bits
ASoC: jz4740-i2s: Use FIELD_PREP() macros in hw_params callback
ASoC: jz4740-i2s: Align macro values and sort includes
ASoC: jz4740-i2s: Make the PLL clock name SoC-specific
ASoC: jz4740-i2s: Support S20_LE and S24_LE sample formats
ASoC: jz4740-i2s: Support continuous sample rate
ASoC: jz4740-i2s: Move component functions near the component driver
ASoC: jz4740-i2s: Refactor DAI probe/remove ops as component ops
sound/soc/jz4740/Kconfig | 1 +
sound/soc/jz4740/jz4740-i2s.c | 461 ++++++++++++++++++----------------
2 files changed, 248 insertions(+), 214 deletions(-)
--
2.35.1
The I2S controller on JZ47xx SoCs doesn't impose restrictions on
sample rate and the driver doesn't make any assumptions about it,
so the DAI should advertise a continuous sample rate range.
Acked-by: Paul Cercueil <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
sound/soc/jz4740/jz4740-i2s.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c
index 9be2f3f1b376..70b9d28a40ce 100644
--- a/sound/soc/jz4740/jz4740-i2s.c
+++ b/sound/soc/jz4740/jz4740-i2s.c
@@ -391,13 +391,13 @@ static struct snd_soc_dai_driver jz4740_i2s_dai = {
.playback = {
.channels_min = 1,
.channels_max = 2,
- .rates = SNDRV_PCM_RATE_8000_48000,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
.formats = JZ4740_I2S_FMTS,
},
.capture = {
.channels_min = 2,
.channels_max = 2,
- .rates = SNDRV_PCM_RATE_8000_48000,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
.formats = JZ4740_I2S_FMTS,
},
.symmetric_rate = 1,
@@ -429,13 +429,13 @@ static struct snd_soc_dai_driver jz4770_i2s_dai = {
.playback = {
.channels_min = 1,
.channels_max = 2,
- .rates = SNDRV_PCM_RATE_8000_48000,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
.formats = JZ4740_I2S_FMTS,
},
.capture = {
.channels_min = 2,
.channels_max = 2,
- .rates = SNDRV_PCM_RATE_8000_48000,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
.formats = JZ4740_I2S_FMTS,
},
.ops = &jz4740_i2s_dai_ops,
--
2.35.1
On some Ingenic SoCs, such as the X1000, there is a programmable
divider used to generate the I2S system clock from a PLL, rather
than a fixed PLL/2 clock. It doesn't make much sense to call the
clock "pll half" on those SoCs, so the clock name should really be
a SoC-dependent value.
Signed-off-by: Aidan MacDonald <[email protected]>
---
sound/soc/jz4740/jz4740-i2s.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c
index 0dcc658b3784..a41398c24d0e 100644
--- a/sound/soc/jz4740/jz4740-i2s.c
+++ b/sound/soc/jz4740/jz4740-i2s.c
@@ -75,6 +75,8 @@ struct i2s_soc_info {
struct reg_field field_i2sdiv_capture;
struct reg_field field_i2sdiv_playback;
+ const char *pll_clk_name;
+
bool shared_fifo_flush;
};
@@ -281,7 +283,7 @@ static int jz4740_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
clk_set_parent(i2s->clk_i2s, parent);
break;
case JZ4740_I2S_CLKSRC_PLL:
- parent = clk_get(NULL, "pll half");
+ parent = clk_get(NULL, i2s->soc_info->pll_clk_name);
if (IS_ERR(parent))
return PTR_ERR(parent);
clk_set_parent(i2s->clk_i2s, parent);
@@ -400,6 +402,7 @@ static const struct i2s_soc_info jz4740_i2s_soc_info = {
.field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 8, 11),
.field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
.field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
+ .pll_clk_name = "pll half",
.shared_fifo_flush = true,
};
@@ -409,6 +412,7 @@ static const struct i2s_soc_info jz4760_i2s_soc_info = {
.field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 16, 20),
.field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
.field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
+ .pll_clk_name = "pll half",
};
static struct snd_soc_dai_driver jz4770_i2s_dai = {
@@ -435,6 +439,7 @@ static const struct i2s_soc_info jz4770_i2s_soc_info = {
.field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 16, 20),
.field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 8, 11),
.field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
+ .pll_clk_name = "pll half",
};
static const struct i2s_soc_info jz4780_i2s_soc_info = {
@@ -443,6 +448,7 @@ static const struct i2s_soc_info jz4780_i2s_soc_info = {
.field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 16, 20),
.field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 8, 11),
.field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
+ .pll_clk_name = "pll half",
};
static const struct snd_soc_component_driver jz4740_i2s_component = {
--
2.35.1
The differences between register fields on different SoC versions
can be abstracted away using the regmap field API. This is easier
to understand and extend than comparisons based on the version ID.
Since the version IDs are unused after this change, remove them at
the same time, and remove unused macros.
Signed-off-by: Aidan MacDonald <[email protected]>
---
sound/soc/jz4740/jz4740-i2s.c | 133 +++++++++++++++++++---------------
1 file changed, 76 insertions(+), 57 deletions(-)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c
index 69ccec0f09d9..bd73427b837e 100644
--- a/sound/soc/jz4740/jz4740-i2s.c
+++ b/sound/soc/jz4740/jz4740-i2s.c
@@ -34,8 +34,6 @@
#define JZ_REG_AIC_CLK_DIV 0x30
#define JZ_REG_AIC_FIFO 0x34
-#define JZ_AIC_CONF_FIFO_RX_THRESHOLD_MASK (0xf << 12)
-#define JZ_AIC_CONF_FIFO_TX_THRESHOLD_MASK (0xf << 8)
#define JZ_AIC_CONF_OVERFLOW_PLAY_LAST BIT(6)
#define JZ_AIC_CONF_INTERNAL_CODEC BIT(5)
#define JZ_AIC_CONF_I2S BIT(4)
@@ -44,11 +42,6 @@
#define JZ_AIC_CONF_SYNC_CLK_MASTER BIT(1)
#define JZ_AIC_CONF_ENABLE BIT(0)
-#define JZ_AIC_CONF_FIFO_RX_THRESHOLD_OFFSET 12
-#define JZ_AIC_CONF_FIFO_TX_THRESHOLD_OFFSET 8
-#define JZ4760_AIC_CONF_FIFO_RX_THRESHOLD_OFFSET 24
-#define JZ4760_AIC_CONF_FIFO_TX_THRESHOLD_OFFSET 16
-
#define JZ_AIC_CTRL_OUTPUT_SAMPLE_SIZE_MASK (0x7 << 19)
#define JZ_AIC_CTRL_INPUT_SAMPLE_SIZE_MASK (0x7 << 16)
#define JZ_AIC_CTRL_ENABLE_RX_DMA BIT(15)
@@ -75,27 +68,23 @@
#define JZ_AIC_I2S_STATUS_BUSY BIT(2)
-#define JZ_AIC_CLK_DIV_MASK 0xf
-#define I2SDIV_DV_SHIFT 0
-#define I2SDIV_DV_MASK (0xf << I2SDIV_DV_SHIFT)
-#define I2SDIV_IDV_SHIFT 8
-#define I2SDIV_IDV_MASK (0xf << I2SDIV_IDV_SHIFT)
-
-enum jz47xx_i2s_version {
- JZ_I2S_JZ4740,
- JZ_I2S_JZ4760,
- JZ_I2S_JZ4770,
- JZ_I2S_JZ4780,
-};
-
struct i2s_soc_info {
- enum jz47xx_i2s_version version;
struct snd_soc_dai_driver *dai;
+
+ struct reg_field field_rx_fifo_thresh;
+ struct reg_field field_tx_fifo_thresh;
+ struct reg_field field_i2sdiv_capture;
+ struct reg_field field_i2sdiv_playback;
};
struct jz4740_i2s {
struct regmap *regmap;
+ struct regmap_field *field_rx_fifo_thresh;
+ struct regmap_field *field_tx_fifo_thresh;
+ struct regmap_field *field_i2sdiv_capture;
+ struct regmap_field *field_i2sdiv_playback;
+
struct clk *clk_aic;
struct clk *clk_i2s;
@@ -217,12 +206,12 @@ static int jz4740_i2s_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
{
struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+ struct regmap_field *div_field;
unsigned int sample_size;
- uint32_t ctrl, div_reg;
+ uint32_t ctrl;
int div;
regmap_read(i2s->regmap, JZ_REG_AIC_CTRL, &ctrl);
- regmap_read(i2s->regmap, JZ_REG_AIC_CLK_DIV, &div_reg);
div = clk_get_rate(i2s->clk_i2s) / (64 * params_rate(params));
@@ -245,23 +234,16 @@ static int jz4740_i2s_hw_params(struct snd_pcm_substream *substream,
else
ctrl &= ~JZ_AIC_CTRL_MONO_TO_STEREO;
- div_reg &= ~I2SDIV_DV_MASK;
- div_reg |= (div - 1) << I2SDIV_DV_SHIFT;
+ div_field = i2s->field_i2sdiv_playback;
} else {
ctrl &= ~JZ_AIC_CTRL_INPUT_SAMPLE_SIZE_MASK;
ctrl |= sample_size << JZ_AIC_CTRL_INPUT_SAMPLE_SIZE_OFFSET;
- if (i2s->soc_info->version >= JZ_I2S_JZ4770) {
- div_reg &= ~I2SDIV_IDV_MASK;
- div_reg |= (div - 1) << I2SDIV_IDV_SHIFT;
- } else {
- div_reg &= ~I2SDIV_DV_MASK;
- div_reg |= (div - 1) << I2SDIV_DV_SHIFT;
- }
+ div_field = i2s->field_i2sdiv_capture;
}
regmap_write(i2s->regmap, JZ_REG_AIC_CTRL, ctrl);
- regmap_write(i2s->regmap, JZ_REG_AIC_CLK_DIV, div_reg);
+ regmap_field_write(div_field, div - 1);
return 0;
}
@@ -334,7 +316,6 @@ static int jz4740_i2s_resume(struct snd_soc_component *component)
static int jz4740_i2s_dai_probe(struct snd_soc_dai *dai)
{
struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai);
- uint32_t conf;
int ret;
ret = clk_prepare_enable(i2s->clk_aic);
@@ -344,22 +325,14 @@ static int jz4740_i2s_dai_probe(struct snd_soc_dai *dai)
snd_soc_dai_init_dma_data(dai, &i2s->playback_dma_data,
&i2s->capture_dma_data);
- if (i2s->soc_info->version >= JZ_I2S_JZ4760) {
- conf = (7 << JZ4760_AIC_CONF_FIFO_RX_THRESHOLD_OFFSET) |
- (8 << JZ4760_AIC_CONF_FIFO_TX_THRESHOLD_OFFSET) |
- JZ_AIC_CONF_OVERFLOW_PLAY_LAST |
- JZ_AIC_CONF_I2S |
- JZ_AIC_CONF_INTERNAL_CODEC;
- } else {
- conf = (7 << JZ_AIC_CONF_FIFO_RX_THRESHOLD_OFFSET) |
- (8 << JZ_AIC_CONF_FIFO_TX_THRESHOLD_OFFSET) |
- JZ_AIC_CONF_OVERFLOW_PLAY_LAST |
- JZ_AIC_CONF_I2S |
- JZ_AIC_CONF_INTERNAL_CODEC;
- }
-
regmap_write(i2s->regmap, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET);
- regmap_write(i2s->regmap, JZ_REG_AIC_CONF, conf);
+
+ regmap_write(i2s->regmap, JZ_REG_AIC_CONF,
+ JZ_AIC_CONF_OVERFLOW_PLAY_LAST |
+ JZ_AIC_CONF_I2S | JZ_AIC_CONF_INTERNAL_CODEC);
+
+ regmap_field_write(i2s->field_rx_fifo_thresh, 7);
+ regmap_field_write(i2s->field_tx_fifo_thresh, 8);
return 0;
}
@@ -404,13 +377,19 @@ static struct snd_soc_dai_driver jz4740_i2s_dai = {
};
static const struct i2s_soc_info jz4740_i2s_soc_info = {
- .version = JZ_I2S_JZ4740,
- .dai = &jz4740_i2s_dai,
+ .dai = &jz4740_i2s_dai,
+ .field_rx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 12, 15),
+ .field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 8, 11),
+ .field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
+ .field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
};
static const struct i2s_soc_info jz4760_i2s_soc_info = {
- .version = JZ_I2S_JZ4760,
- .dai = &jz4740_i2s_dai,
+ .dai = &jz4740_i2s_dai,
+ .field_rx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 24, 27),
+ .field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 16, 20),
+ .field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
+ .field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
};
static struct snd_soc_dai_driver jz4770_i2s_dai = {
@@ -432,13 +411,19 @@ static struct snd_soc_dai_driver jz4770_i2s_dai = {
};
static const struct i2s_soc_info jz4770_i2s_soc_info = {
- .version = JZ_I2S_JZ4770,
- .dai = &jz4770_i2s_dai,
+ .dai = &jz4770_i2s_dai,
+ .field_rx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 24, 27),
+ .field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 16, 20),
+ .field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 8, 11),
+ .field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
};
static const struct i2s_soc_info jz4780_i2s_soc_info = {
- .version = JZ_I2S_JZ4780,
- .dai = &jz4770_i2s_dai,
+ .dai = &jz4770_i2s_dai,
+ .field_rx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 24, 27),
+ .field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 16, 20),
+ .field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 8, 11),
+ .field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3),
};
static const struct snd_soc_component_driver jz4740_i2s_component = {
@@ -457,6 +442,36 @@ static const struct of_device_id jz4740_of_matches[] = {
};
MODULE_DEVICE_TABLE(of, jz4740_of_matches);
+static int jz4740_i2s_init_regmap_fields(struct device *dev,
+ struct jz4740_i2s *i2s)
+{
+ i2s->field_rx_fifo_thresh =
+ devm_regmap_field_alloc(dev, i2s->regmap,
+ i2s->soc_info->field_rx_fifo_thresh);
+ if (IS_ERR(i2s->field_rx_fifo_thresh))
+ return PTR_ERR(i2s->field_rx_fifo_thresh);
+
+ i2s->field_tx_fifo_thresh =
+ devm_regmap_field_alloc(dev, i2s->regmap,
+ i2s->soc_info->field_tx_fifo_thresh);
+ if (IS_ERR(i2s->field_tx_fifo_thresh))
+ return PTR_ERR(i2s->field_tx_fifo_thresh);
+
+ i2s->field_i2sdiv_capture =
+ devm_regmap_field_alloc(dev, i2s->regmap,
+ i2s->soc_info->field_i2sdiv_capture);
+ if (IS_ERR(i2s->field_i2sdiv_capture))
+ return PTR_ERR(i2s->field_i2sdiv_capture);
+
+ i2s->field_i2sdiv_playback =
+ devm_regmap_field_alloc(dev, i2s->regmap,
+ i2s->soc_info->field_i2sdiv_playback);
+ if (IS_ERR(i2s->field_i2sdiv_playback))
+ return PTR_ERR(i2s->field_i2sdiv_playback);
+
+ return 0;
+}
+
static const struct regmap_config jz4740_i2s_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
@@ -501,6 +516,10 @@ static int jz4740_i2s_dev_probe(struct platform_device *pdev)
if (IS_ERR(i2s->regmap))
return PTR_ERR(i2s->regmap);
+ ret = jz4740_i2s_init_regmap_fields(dev, i2s);
+ if (ret)
+ return ret;
+
platform_set_drvdata(pdev, i2s);
ret = devm_snd_soc_register_component(dev, &jz4740_i2s_component,
--
2.35.1
The audio controller on JZ47xx SoCs can transfer 20- and 24-bit
samples in the FIFO, so allow those formats to be used with the
I2S driver. Although the FIFO doesn't care about the in-memory
sample format, we only support 4-byte format variants because the
DMA controller on these SoCs cannot transfer in 3-byte multiples.
Signed-off-by: Aidan MacDonald <[email protected]>
---
sound/soc/jz4740/jz4740-i2s.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c
index a41398c24d0e..9be2f3f1b376 100644
--- a/sound/soc/jz4740/jz4740-i2s.c
+++ b/sound/soc/jz4740/jz4740-i2s.c
@@ -238,9 +238,15 @@ static int jz4740_i2s_hw_params(struct snd_pcm_substream *substream,
case SNDRV_PCM_FORMAT_S8:
sample_size = 0;
break;
- case SNDRV_PCM_FORMAT_S16:
+ case SNDRV_PCM_FORMAT_S16_LE:
sample_size = 1;
break;
+ case SNDRV_PCM_FORMAT_S20_LE:
+ sample_size = 3;
+ break;
+ case SNDRV_PCM_FORMAT_S24_LE:
+ sample_size = 4;
+ break;
default:
return -EINVAL;
}
@@ -375,7 +381,9 @@ static const struct snd_soc_dai_ops jz4740_i2s_dai_ops = {
};
#define JZ4740_I2S_FMTS (SNDRV_PCM_FMTBIT_S8 | \
- SNDRV_PCM_FMTBIT_S16_LE)
+ SNDRV_PCM_FMTBIT_S16_LE | \
+ SNDRV_PCM_FMTBIT_S20_LE | \
+ SNDRV_PCM_FMTBIT_S24_LE)
static struct snd_soc_dai_driver jz4740_i2s_dai = {
.probe = jz4740_i2s_dai_probe,
--
2.35.1