Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1681612 > unrolled thread
| Started by | codekipper@gmail.com |
|---|---|
| First post | 2017-07-05 17:50 +0200 |
| Last post | 2017-07-06 04:10 +0200 |
| Articles | 5 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH 0/3] ASoC: Add I2S support for Allwinner H3 SoCs codekipper@gmail.com - 2017-07-05 17:50 +0200
[PATCH 3/3] ASoC: sun4i-i2s: Add support for H3 codekipper@gmail.com - 2017-07-05 17:50 +0200
Re: [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3 Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-05 18:30 +0200
Re: [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3 Code Kipper <codekipper@gmail.com> - 2017-07-05 19:50 +0200
Re: [linux-sunxi] Re: [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3 Chen-Yu Tsai <wens@csie.org> - 2017-07-06 04:10 +0200
| From | codekipper@gmail.com |
|---|---|
| Date | 2017-07-05 17:50 +0200 |
| Subject | [PATCH 0/3] ASoC: Add I2S support for Allwinner H3 SoCs |
| Message-ID | <tZUIV-89P-3@gated-at.bofh.it> |
From: Marcus Cooper <codekipper@gmail.com> Hi All, please find attached a series of patches to bring i2s support to the Allwinner H3 SoC. This has been tested with the following setups: A20 Olimex EVB connected to a pcm5102 Orange Pi 2 connected to a uda1380 Orange Pi 2 hdmi audio playback Pine 64 connected to the audio DAC board To get i2s working some additional patches are required which will be delivered later. For now they have been pushed here https://github.com/codekipper/linux-sunxi/commits/sunxi-audio-h3 Thanks in advance, CK Marcus Cooper (3): ASoC: sun4i-i2s: Add more quirks for newer SoCs ASoC: sun4i-i2s: Get startup to call set_fmt ASoC: sun4i-i2s: Add support for H3 .../devicetree/bindings/sound/sun4i-i2s.txt | 2 + sound/soc/sunxi/sun4i-i2s.c | 391 ++++++++++++++++++++- 2 files changed, 376 insertions(+), 17 deletions(-) -- 2.13.2
[toc] | [next] | [standalone]
| From | codekipper@gmail.com |
|---|---|
| Date | 2017-07-05 17:50 +0200 |
| Subject | [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3 |
| Message-ID | <tZUIW-89P-25@gated-at.bofh.it> |
| In reply to | #1681612 |
From: Marcus Cooper <codekipper@gmail.com>
There are a lot of changes to the sun8i-h3 i2s block but not enough
to warrant to a new driver.
Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
.../devicetree/bindings/sound/sun4i-i2s.txt | 2 +
sound/soc/sunxi/sun4i-i2s.c | 339 ++++++++++++++++++++-
2 files changed, 337 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
index ee21da865771..fc5da6080759 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
@@ -8,6 +8,7 @@ Required properties:
- compatible: should be one of the following:
- "allwinner,sun4i-a10-i2s"
- "allwinner,sun6i-a31-i2s"
+ - "allwinner,sun8i-h3-i2s"
- reg: physical base address of the controller and length of memory mapped
region.
- interrupts: should contain the I2S interrupt.
@@ -22,6 +23,7 @@ Required properties:
Required properties for the following compatibles:
- "allwinner,sun6i-a31-i2s"
+ - "allwinner,sun8i-h3-i2s"
- resets: phandle to the reset line for this codec
Example:
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index bb7affd53002..0b853fe320e0 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -26,9 +26,16 @@
#include <sound/soc-dai.h>
#define SUN4I_I2S_CTRL_REG 0x00
+#define SUN4I_I2S_CTRL_BCLK_OUT BIT(18)
+#define SUN4I_I2S_CTRL_LRCK_OUT BIT(17)
+#define SUN4I_I2S_CTRL_LRCKR_OUT BIT(16)
#define SUN4I_I2S_CTRL_SDO_EN_MASK GENMASK(11, 8)
#define SUN4I_I2S_CTRL_SDO_EN(sdo) BIT(8 + (sdo))
#define SUN4I_I2S_CTRL_MODE_MASK BIT(5)
+#define SUN8I_I2S_CTRL_MODE_MASK GENMASK(5, 4)
+#define SUN8I_I2S_CTRL_MODE_RIGHT_J (2 << 4)
+#define SUN8I_I2S_CTRL_MODE_I2S (1 << 4)
+#define SUN8I_I2S_CTRL_MODE_PCM (0 << 4)
#define SUN4I_I2S_CTRL_MODE_SLAVE (1 << 5)
#define SUN4I_I2S_CTRL_MODE_MASTER (0 << 5)
#define SUN4I_I2S_CTRL_TX_EN BIT(2)
@@ -36,16 +43,27 @@
#define SUN4I_I2S_CTRL_GL_EN BIT(0)
#define SUN4I_I2S_FMT0_REG 0x04
+#define SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK BIT(19)
+#define SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED (1 << 19)
+#define SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL (0 << 19)
+#define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK GENMASK(17, 8)
+#define SUN8I_I2S_FMT0_LRCK_PERIOD(period) ((period) << 8)
#define SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK BIT(7)
#define SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED (1 << 7)
#define SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL (0 << 7)
+#define SUN8I_I2S_FMT0_BCLK_POLARITY_MASK BIT(7)
+#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 7)
+#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 7)
#define SUN4I_I2S_FMT0_BCLK_POLARITY_MASK BIT(6)
#define SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 6)
#define SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 6)
#define SUN4I_I2S_FMT0_SR_MASK GENMASK(5, 4)
+#define SUN8I_I2S_FMT0_SR_MASK GENMASK(6, 4)
#define SUN4I_I2S_FMT0_SR(sr) ((sr) << 4)
#define SUN4I_I2S_FMT0_WSS_MASK GENMASK(3, 2)
#define SUN4I_I2S_FMT0_WSS(wss) ((wss) << 2)
+#define SUN8I_I2S_FMT0_WSS_MASK GENMASK(2, 0)
+#define SUN8I_I2S_FMT0_WSS(wss) (wss)
#define SUN4I_I2S_FMT0_FMT_MASK GENMASK(1, 0)
#define SUN4I_I2S_FMT0_FMT_RIGHT_J (2 << 0)
#define SUN4I_I2S_FMT0_FMT_LEFT_J (1 << 0)
@@ -53,6 +71,7 @@
#define SUN4I_I2S_FMT1_REG 0x08
#define SUN4I_I2S_FIFO_TX_REG 0x0c
+#define SUN8I_I2S_INT_STA_REG 0x0c
#define SUN4I_I2S_FIFO_RX_REG 0x10
#define SUN4I_I2S_FIFO_CTRL_REG 0x14
@@ -70,10 +89,13 @@
#define SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN BIT(3)
#define SUN4I_I2S_INT_STA_REG 0x20
+#define SUN8I_I2S_FIFO_TX_REG 0x20
#define SUN4I_I2S_CLK_DIV_REG 0x24
+#define SUN8I_I2S_CLK_DIV_MCLK_EN BIT(8)
#define SUN4I_I2S_CLK_DIV_MCLK_EN BIT(7)
#define SUN4I_I2S_CLK_DIV_BCLK_MASK GENMASK(6, 4)
+#define SUN8I_I2S_CLK_DIV_BCLK_MASK GENMASK(7, 4)
#define SUN4I_I2S_CLK_DIV_BCLK(bclk) ((bclk) << 4)
#define SUN4I_I2S_CLK_DIV_MCLK_MASK GENMASK(3, 0)
#define SUN4I_I2S_CLK_DIV_MCLK(mclk) ((mclk) << 0)
@@ -82,14 +104,27 @@
#define SUN4I_I2S_TX_CNT_REG 0x2c
#define SUN4I_I2S_TX_CHAN_SEL_REG 0x30
+#define SUN8I_I2S_TX_CHAN_CFG_REG 0x30
#define SUN4I_I2S_TX_CHAN_SEL(num_chan) (((num_chan) - 1) << 0)
#define SUN4I_I2S_TX_CHAN_MAP_REG 0x34
#define SUN4I_I2S_TX_CHAN_MAP(chan, sample) ((sample) << (chan << 2))
+#define SUN8I_I2S_TX_CHAN_SEL_REG 0x34
+#define SUN8I_I2S_TX_CHAN_OFFSET_MASK GENMASK(13, 12)
+#define SUN8I_I2S_TX_CHAN_OFFSET(offset) (offset << 12)
+#define SUN8I_I2S_TX_CHAN_EN_MASK GENMASK(11, 4)
+#define SUN8I_I2S_TX_CHAN_EN(num_chan) (((1 << num_chan) - 1) << 4)
+#define SUN8I_I2S_TX_CHAN_SEL_MASK GENMASK(2, 0)
+#define SUN8I_I2S_TX_CHAN_SEL(num_chan) (((num_chan) - 1) << 0)
#define SUN4I_I2S_RX_CHAN_SEL_REG 0x38
#define SUN4I_I2S_RX_CHAN_MAP_REG 0x3c
+#define SUN8I_I2S_TX_CHAN_MAP_REG 0x44
+
+#define SUN8I_I2S_RX_CHAN_SEL_REG 0x54
+#define SUN8I_I2S_RX_CHAN_MAP_REG 0x58
+
struct sun4i_i2s {
struct clk *bus_clk;
struct clk *mod_clk;
@@ -363,6 +398,225 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
return 0;
}
+static int sun8i_i2s_set_clk_rate(struct sun4i_i2s *i2s,
+ unsigned int rate,
+ unsigned int word_size)
+{
+ unsigned int clk_rate;
+ int bclk_div, mclk_div;
+ int ret, i;
+
+ switch (rate) {
+ case 176400:
+ case 88200:
+ case 44100:
+ case 22050:
+ case 11025:
+ clk_rate = 22579200;
+ break;
+
+ case 192000:
+ case 128000:
+ case 96000:
+ case 64000:
+ case 48000:
+ case 32000:
+ case 24000:
+ case 16000:
+ case 12000:
+ case 8000:
+ clk_rate = 24576000;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ ret = clk_set_rate(i2s->mod_clk, clk_rate);
+ if (ret)
+ return ret;
+
+ /* Always favor the highest oversampling rate */
+ for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) {
+ unsigned int oversample_rate = sun4i_i2s_oversample_rates[i];
+
+ bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
+ word_size);
+ mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
+ clk_rate,
+ rate);
+
+ if ((bclk_div >= 0) && (mclk_div >= 0))
+ break;
+ }
+
+ if ((bclk_div < 0) || (mclk_div < 0))
+ return -EINVAL;
+
+ regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG,
+ SUN4I_I2S_CLK_DIV_BCLK(bclk_div) |
+ SUN4I_I2S_CLK_DIV_MCLK(mclk_div + 1) |
+ SUN8I_I2S_CLK_DIV_MCLK_EN);
+
+ /* Set sync period */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+ SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
+ SUN8I_I2S_FMT0_LRCK_PERIOD((2 * word_size)-1));
+ regmap_write(i2s->regmap, SUN4I_I2S_FMT1_REG, 0);
+
+ return 0;
+}
+
+static int sun8i_i2s_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+ int sr, wss;
+ u32 width, channels = 0;
+
+ switch (params_channels(params)) {
+ case 2:
+ channels |= SUN4I_I2S_CTRL_SDO_EN(0);
+ break;
+ default:
+ dev_err(dai->dev, "invalid channel: %d\n",
+ params_channels(params));
+ return -EINVAL;
+ }
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+ SUN4I_I2S_CTRL_SDO_EN_MASK, channels);
+
+ /* Configure the channels */
+ regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_CFG_REG,
+ SUN4I_I2S_TX_CHAN_SEL(params_channels(params)));
+
+ /* Select the channels */
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+ SUN8I_I2S_TX_CHAN_EN_MASK |
+ SUN8I_I2S_TX_CHAN_SEL_MASK,
+ SUN8I_I2S_TX_CHAN_EN(params_channels(params)) |
+ SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
+
+ /* Map the channels for stereo playback */
+ regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG,
+ SUN4I_I2S_TX_CHAN_MAP(1, 1));
+
+ /* Select the channels */
+ regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
+ SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
+
+ /* Map the channels for stereo capture */
+ regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG,
+ SUN4I_I2S_TX_CHAN_MAP(1, 1));
+
+ switch (params_physical_width(params)) {
+ case 16:
+ width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+ break;
+ case 24:
+ case 32:
+ width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ break;
+ default:
+ return -EINVAL;
+ }
+ i2s->playback_dma_data.addr_width = width;
+
+ switch (params_width(params)) {
+ case 16:
+ sr = 3;
+ wss = 3;
+ break;
+ case 20:
+ sr = 4;
+ wss = 4;
+ break;
+ case 24:
+ sr = 5;
+ wss = 5;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+ SUN8I_I2S_FMT0_WSS_MASK | SUN8I_I2S_FMT0_SR_MASK,
+ SUN8I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
+
+ return sun8i_i2s_set_clk_rate(i2s, params_rate(params),
+ params_width(params));
+}
+
+static int sun8i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+ struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+ u32 val = SUN8I_I2S_CTRL_MODE_I2S;
+ u32 offset = 0;
+
+ /* DAI Mode */
+ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_I2S:
+ offset = 1;
+ break;
+ case SND_SOC_DAIFMT_LEFT_J:
+ break;
+ case SND_SOC_DAIFMT_RIGHT_J:
+ val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+ SUN8I_I2S_CTRL_MODE_MASK,
+ val);
+
+ /* blck offset determines whether i2s or LJ */
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+ SUN8I_I2S_TX_CHAN_OFFSET_MASK,
+ SUN8I_I2S_TX_CHAN_OFFSET(offset));
+
+ /* DAI clock polarity */
+ switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+ case SND_SOC_DAIFMT_IB_IF:
+ /* Invert both clocks */
+ val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
+ SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
+ break;
+ case SND_SOC_DAIFMT_IB_NF:
+ /* Invert bit clock */
+ val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
+ SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
+ break;
+ case SND_SOC_DAIFMT_NB_IF:
+ /* Invert frame clock */
+ val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
+ SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;
+ break;
+ case SND_SOC_DAIFMT_NB_NF:
+ /* Nothing to do for both normal cases */
+ val = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL |
+ SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+ SUN8I_I2S_FMT0_BCLK_POLARITY_MASK |
+ SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK,
+ val);
+
+ /* Set significant bits in our FIFOs */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
+ return 0;
+}
+
static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
{
/* Flush RX FIFO */
@@ -471,8 +725,8 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
int ret = 0;
/* Enable the whole hardware block */
- regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
- SUN4I_I2S_CTRL_GL_EN);
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+ SUN4I_I2S_CTRL_GL_EN, SUN4I_I2S_CTRL_GL_EN);
/* Enable the first output line */
regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
@@ -500,7 +754,8 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
SUN4I_I2S_CTRL_SDO_EN_MASK, 0);
/* Disable the whole hardware block */
- regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0);
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+ SUN4I_I2S_CTRL_GL_EN, 0);
}
static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
@@ -525,6 +780,15 @@ static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
.trigger = sun4i_i2s_trigger,
};
+static const struct snd_soc_dai_ops sun8i_i2s_dai_ops = {
+ .hw_params = sun8i_i2s_hw_params,
+ .set_fmt = sun8i_i2s_set_fmt,
+ .set_sysclk = sun4i_i2s_set_sysclk,
+ .shutdown = sun4i_i2s_shutdown,
+ .startup = sun4i_i2s_startup,
+ .trigger = sun4i_i2s_trigger,
+};
+
static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
{
struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
@@ -572,11 +836,11 @@ static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
}
}
+
static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
case SUN4I_I2S_FIFO_RX_REG:
- case SUN4I_I2S_FIFO_STA_REG:
return false;
default:
@@ -588,6 +852,7 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
case SUN4I_I2S_FIFO_RX_REG:
+ case SUN4I_I2S_FIFO_STA_REG:
case SUN4I_I2S_INT_STA_REG:
case SUN4I_I2S_RX_CNT_REG:
case SUN4I_I2S_TX_CNT_REG:
@@ -598,6 +863,34 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
}
}
+static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case SUN8I_I2S_FIFO_TX_REG:
+ return false;
+
+ default:
+ return true;
+ }
+}
+
+static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case SUN4I_I2S_FIFO_RX_REG:
+ case SUN4I_I2S_FIFO_CTRL_REG:
+ case SUN4I_I2S_FIFO_STA_REG:
+ case SUN8I_I2S_INT_STA_REG:
+ case SUN4I_I2S_RX_CNT_REG:
+ case SUN4I_I2S_TX_CNT_REG:
+ case SUN4I_I2S_TX_CHAN_MAP_REG:
+ return true;
+
+ default:
+ return false;
+ }
+}
+
static const struct reg_default sun4i_i2s_reg_defaults[] = {
{ SUN4I_I2S_CTRL_REG, 0x00000000 },
{ SUN4I_I2S_FMT0_REG, 0x0000000c },
@@ -611,6 +904,20 @@ static const struct reg_default sun4i_i2s_reg_defaults[] = {
{ SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
};
+static const struct reg_default sun8i_i2s_reg_defaults[] = {
+ { SUN4I_I2S_CTRL_REG, 0x00060000 },
+ { SUN4I_I2S_FMT0_REG, 0x00000033 },
+ { SUN4I_I2S_FMT1_REG, 0x00000030 },
+ { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
+ { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
+ { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
+ { SUN8I_I2S_TX_CHAN_CFG_REG, 0x00000000 },
+ { SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
+ { SUN8I_I2S_TX_CHAN_MAP_REG, 0x00000000 },
+ { SUN8I_I2S_RX_CHAN_SEL_REG, 0x00000000 },
+ { SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
+};
+
static const struct regmap_config sun4i_i2s_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
@@ -625,6 +932,19 @@ static const struct regmap_config sun4i_i2s_regmap_config = {
.volatile_reg = sun4i_i2s_volatile_reg,
};
+static const struct regmap_config sun8i_i2s_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = SUN8I_I2S_RX_CHAN_MAP_REG,
+ .cache_type = REGCACHE_FLAT,
+ .reg_defaults = sun8i_i2s_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(sun8i_i2s_reg_defaults),
+ .writeable_reg = sun4i_i2s_wr_reg,
+ .readable_reg = sun8i_i2s_rd_reg,
+ .volatile_reg = sun8i_i2s_volatile_reg,
+};
+
static int sun4i_i2s_runtime_resume(struct device *dev)
{
struct sun4i_i2s *i2s = dev_get_drvdata(dev);
@@ -684,6 +1004,13 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
.ops = &sun4i_i2s_dai_ops,
};
+static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
+ .has_reset = true,
+ .reg_offset_txdata = SUN8I_I2S_FIFO_TX_REG,
+ .sun4i_i2s_regmap = &sun8i_i2s_regmap_config,
+ .ops = &sun8i_i2s_dai_ops,
+};
+
static int sun4i_i2s_probe(struct platform_device *pdev)
{
struct sun4i_i2s *i2s;
@@ -823,6 +1150,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
.compatible = "allwinner,sun6i-a31-i2s",
.data = &sun6i_a31_i2s_quirks,
},
+ {
+ .compatible = "allwinner,sun8i-h3-i2s",
+ .data = &sun8i_h3_i2s_quirks,
+ },
{}
};
MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
--
2.13.2
[toc] | [prev] | [next] | [standalone]
| From | Maxime Ripard <maxime.ripard@free-electrons.com> |
|---|---|
| Date | 2017-07-05 18:30 +0200 |
| Subject | Re: [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3 |
| Message-ID | <tZVlE-aF-13@gated-at.bofh.it> |
| In reply to | #1681614 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Jul 05, 2017 at 05:43:24PM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> There are a lot of changes to the sun8i-h3 i2s block but not enough
> to warrant to a new driver.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
> .../devicetree/bindings/sound/sun4i-i2s.txt | 2 +
> sound/soc/sunxi/sun4i-i2s.c | 339 ++++++++++++++++++++-
> 2 files changed, 337 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> index ee21da865771..fc5da6080759 100644
> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> @@ -8,6 +8,7 @@ Required properties:
> - compatible: should be one of the following:
> - "allwinner,sun4i-a10-i2s"
> - "allwinner,sun6i-a31-i2s"
> + - "allwinner,sun8i-h3-i2s"
> - reg: physical base address of the controller and length of memory mapped
> region.
> - interrupts: should contain the I2S interrupt.
> @@ -22,6 +23,7 @@ Required properties:
>
> Required properties for the following compatibles:
> - "allwinner,sun6i-a31-i2s"
> + - "allwinner,sun8i-h3-i2s"
> - resets: phandle to the reset line for this codec
>
> Example:
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index bb7affd53002..0b853fe320e0 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -26,9 +26,16 @@
> #include <sound/soc-dai.h>
>
> #define SUN4I_I2S_CTRL_REG 0x00
> +#define SUN4I_I2S_CTRL_BCLK_OUT BIT(18)
> +#define SUN4I_I2S_CTRL_LRCK_OUT BIT(17)
> +#define SUN4I_I2S_CTRL_LRCKR_OUT BIT(16)
> #define SUN4I_I2S_CTRL_SDO_EN_MASK GENMASK(11, 8)
> #define SUN4I_I2S_CTRL_SDO_EN(sdo) BIT(8 + (sdo))
> #define SUN4I_I2S_CTRL_MODE_MASK BIT(5)
> +#define SUN8I_I2S_CTRL_MODE_MASK GENMASK(5, 4)
> +#define SUN8I_I2S_CTRL_MODE_RIGHT_J (2 << 4)
> +#define SUN8I_I2S_CTRL_MODE_I2S (1 << 4)
> +#define SUN8I_I2S_CTRL_MODE_PCM (0 << 4)
> #define SUN4I_I2S_CTRL_MODE_SLAVE (1 << 5)
> #define SUN4I_I2S_CTRL_MODE_MASTER (0 << 5)
> #define SUN4I_I2S_CTRL_TX_EN BIT(2)
> @@ -36,16 +43,27 @@
> #define SUN4I_I2S_CTRL_GL_EN BIT(0)
>
> #define SUN4I_I2S_FMT0_REG 0x04
> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK BIT(19)
> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED (1 << 19)
> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL (0 << 19)
> +#define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK GENMASK(17, 8)
> +#define SUN8I_I2S_FMT0_LRCK_PERIOD(period) ((period) << 8)
> #define SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK BIT(7)
> #define SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED (1 << 7)
> #define SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL (0 << 7)
> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_MASK BIT(7)
> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 7)
> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 7)
> #define SUN4I_I2S_FMT0_BCLK_POLARITY_MASK BIT(6)
> #define SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 6)
> #define SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 6)
> #define SUN4I_I2S_FMT0_SR_MASK GENMASK(5, 4)
> +#define SUN8I_I2S_FMT0_SR_MASK GENMASK(6, 4)
> #define SUN4I_I2S_FMT0_SR(sr) ((sr) << 4)
> #define SUN4I_I2S_FMT0_WSS_MASK GENMASK(3, 2)
> #define SUN4I_I2S_FMT0_WSS(wss) ((wss) << 2)
> +#define SUN8I_I2S_FMT0_WSS_MASK GENMASK(2, 0)
> +#define SUN8I_I2S_FMT0_WSS(wss) (wss)
> #define SUN4I_I2S_FMT0_FMT_MASK GENMASK(1, 0)
> #define SUN4I_I2S_FMT0_FMT_RIGHT_J (2 << 0)
> #define SUN4I_I2S_FMT0_FMT_LEFT_J (1 << 0)
> @@ -53,6 +71,7 @@
>
> #define SUN4I_I2S_FMT1_REG 0x08
> #define SUN4I_I2S_FIFO_TX_REG 0x0c
> +#define SUN8I_I2S_INT_STA_REG 0x0c
> #define SUN4I_I2S_FIFO_RX_REG 0x10
>
> #define SUN4I_I2S_FIFO_CTRL_REG 0x14
> @@ -70,10 +89,13 @@
> #define SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN BIT(3)
>
> #define SUN4I_I2S_INT_STA_REG 0x20
> +#define SUN8I_I2S_FIFO_TX_REG 0x20
>
> #define SUN4I_I2S_CLK_DIV_REG 0x24
> +#define SUN8I_I2S_CLK_DIV_MCLK_EN BIT(8)
> #define SUN4I_I2S_CLK_DIV_MCLK_EN BIT(7)
> #define SUN4I_I2S_CLK_DIV_BCLK_MASK GENMASK(6, 4)
> +#define SUN8I_I2S_CLK_DIV_BCLK_MASK GENMASK(7, 4)
> #define SUN4I_I2S_CLK_DIV_BCLK(bclk) ((bclk) << 4)
> #define SUN4I_I2S_CLK_DIV_MCLK_MASK GENMASK(3, 0)
> #define SUN4I_I2S_CLK_DIV_MCLK(mclk) ((mclk) << 0)
> @@ -82,14 +104,27 @@
> #define SUN4I_I2S_TX_CNT_REG 0x2c
>
> #define SUN4I_I2S_TX_CHAN_SEL_REG 0x30
> +#define SUN8I_I2S_TX_CHAN_CFG_REG 0x30
> #define SUN4I_I2S_TX_CHAN_SEL(num_chan) (((num_chan) - 1) << 0)
>
> #define SUN4I_I2S_TX_CHAN_MAP_REG 0x34
> #define SUN4I_I2S_TX_CHAN_MAP(chan, sample) ((sample) << (chan << 2))
> +#define SUN8I_I2S_TX_CHAN_SEL_REG 0x34
> +#define SUN8I_I2S_TX_CHAN_OFFSET_MASK GENMASK(13, 12)
> +#define SUN8I_I2S_TX_CHAN_OFFSET(offset) (offset << 12)
> +#define SUN8I_I2S_TX_CHAN_EN_MASK GENMASK(11, 4)
> +#define SUN8I_I2S_TX_CHAN_EN(num_chan) (((1 << num_chan) - 1) << 4)
> +#define SUN8I_I2S_TX_CHAN_SEL_MASK GENMASK(2, 0)
> +#define SUN8I_I2S_TX_CHAN_SEL(num_chan) (((num_chan) - 1) << 0)
>
> #define SUN4I_I2S_RX_CHAN_SEL_REG 0x38
> #define SUN4I_I2S_RX_CHAN_MAP_REG 0x3c
>
> +#define SUN8I_I2S_TX_CHAN_MAP_REG 0x44
> +
> +#define SUN8I_I2S_RX_CHAN_SEL_REG 0x54
> +#define SUN8I_I2S_RX_CHAN_MAP_REG 0x58
> +
> struct sun4i_i2s {
> struct clk *bus_clk;
> struct clk *mod_clk;
> @@ -363,6 +398,225 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> return 0;
> }
>
> +static int sun8i_i2s_set_clk_rate(struct sun4i_i2s *i2s,
> + unsigned int rate,
> + unsigned int word_size)
> +{
> + unsigned int clk_rate;
> + int bclk_div, mclk_div;
> + int ret, i;
> +
> + switch (rate) {
> + case 176400:
> + case 88200:
> + case 44100:
> + case 22050:
> + case 11025:
> + clk_rate = 22579200;
> + break;
> +
> + case 192000:
> + case 128000:
> + case 96000:
> + case 64000:
> + case 48000:
> + case 32000:
> + case 24000:
> + case 16000:
> + case 12000:
> + case 8000:
> + clk_rate = 24576000;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + ret = clk_set_rate(i2s->mod_clk, clk_rate);
> + if (ret)
> + return ret;
> +
> + /* Always favor the highest oversampling rate */
> + for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) {
> + unsigned int oversample_rate = sun4i_i2s_oversample_rates[i];
> +
> + bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
> + word_size);
> + mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
> + clk_rate,
> + rate);
> +
> + if ((bclk_div >= 0) && (mclk_div >= 0))
> + break;
> + }
> +
> + if ((bclk_div < 0) || (mclk_div < 0))
> + return -EINVAL;
> +
> + regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG,
> + SUN4I_I2S_CLK_DIV_BCLK(bclk_div) |
> + SUN4I_I2S_CLK_DIV_MCLK(mclk_div + 1) |
> + SUN8I_I2S_CLK_DIV_MCLK_EN);
Duplicating all that code just for a single bit difference doesn't
seem very wise. You can handle that bit difference through
regmap_fields.
> + /* Set sync period */
> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> + SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
> + SUN8I_I2S_FMT0_LRCK_PERIOD((2 * word_size)-1));
> + regmap_write(i2s->regmap, SUN4I_I2S_FMT1_REG, 0);
It seems to be applicable for the old flavour too, why isn't it there?
> +
> + return 0;
> +}
> +
> +static int sun8i_i2s_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *dai)
> +{
> + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> + int sr, wss;
> + u32 width, channels = 0;
> +
> + switch (params_channels(params)) {
> + case 2:
> + channels |= SUN4I_I2S_CTRL_SDO_EN(0);
> + break;
> + default:
> + dev_err(dai->dev, "invalid channel: %d\n",
> + params_channels(params));
> + return -EINVAL;
> + }
> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> + SUN4I_I2S_CTRL_SDO_EN_MASK, channels);
This seems applicable to the old generation.
> + /* Configure the channels */
> + regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_CFG_REG,
> + SUN4I_I2S_TX_CHAN_SEL(params_channels(params)));
This can be handled by regmap_fields.
> + /* Select the channels */
> + regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> + SUN8I_I2S_TX_CHAN_EN_MASK |
> + SUN8I_I2S_TX_CHAN_SEL_MASK,
> + SUN8I_I2S_TX_CHAN_EN(params_channels(params)) |
> + SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
Ditto.
>
> + /* Map the channels for stereo playback */
> + regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG,
> + SUN4I_I2S_TX_CHAN_MAP(1, 1));
Ditto.
> + /* Select the channels */
> + regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
> + SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
Ditto.
> + /* Map the channels for stereo capture */
> + regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG,
> + SUN4I_I2S_TX_CHAN_MAP(1, 1));
Ditto.
> + switch (params_physical_width(params)) {
> + case 16:
> + width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + break;
> + case 24:
> + case 32:
> + width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + break;
> + default:
> + return -EINVAL;
> + }
> + i2s->playback_dma_data.addr_width = width;
This is applicable to the old generation.
> +
> + switch (params_width(params)) {
> + case 16:
> + sr = 3;
> + wss = 3;
> + break;
> + case 20:
> + sr = 4;
> + wss = 4;
> + break;
> + case 24:
> + sr = 5;
> + wss = 5;
> + break;
> + default:
> + return -EINVAL;
> + }
It looks like this changed, maybe we can split it into a separate
function?
> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> + SUN8I_I2S_FMT0_WSS_MASK | SUN8I_I2S_FMT0_SR_MASK,
> + SUN8I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
> +
> + return sun8i_i2s_set_clk_rate(i2s, params_rate(params),
> + params_width(params));
> +}
> +
> +static int sun8i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> +{
> + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> + u32 val = SUN8I_I2S_CTRL_MODE_I2S;
> + u32 offset = 0;
> +
> + /* DAI Mode */
> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + case SND_SOC_DAIFMT_I2S:
> + offset = 1;
> + break;
> + case SND_SOC_DAIFMT_LEFT_J:
> + break;
> + case SND_SOC_DAIFMT_RIGHT_J:
> + val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> + SUN8I_I2S_CTRL_MODE_MASK,
> + val);
> +
> + /* blck offset determines whether i2s or LJ */
> + regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> + SUN8I_I2S_TX_CHAN_OFFSET_MASK,
> + SUN8I_I2S_TX_CHAN_OFFSET(offset));
> +
> + /* DAI clock polarity */
> + switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> + case SND_SOC_DAIFMT_IB_IF:
> + /* Invert both clocks */
> + val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
> + SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
> + break;
> + case SND_SOC_DAIFMT_IB_NF:
> + /* Invert bit clock */
> + val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
> + SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
> + break;
> + case SND_SOC_DAIFMT_NB_IF:
> + /* Invert frame clock */
> + val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
> + SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;
> + break;
> + case SND_SOC_DAIFMT_NB_NF:
> + /* Nothing to do for both normal cases */
> + val = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL |
> + SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> + SUN8I_I2S_FMT0_BCLK_POLARITY_MASK |
> + SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK,
> + val);
> +
> + /* Set significant bits in our FIFOs */
> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
> + SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
> + SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
> + SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
> + SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
> + return 0;
> +}
> +
> static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
> {
> /* Flush RX FIFO */
> @@ -471,8 +725,8 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
> int ret = 0;
>
> /* Enable the whole hardware block */
> - regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
> - SUN4I_I2S_CTRL_GL_EN);
> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> + SUN4I_I2S_CTRL_GL_EN, SUN4I_I2S_CTRL_GL_EN);
Why? This needs to be explained.
>
> /* Enable the first output line */
> regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> @@ -500,7 +754,8 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
> SUN4I_I2S_CTRL_SDO_EN_MASK, 0);
>
> /* Disable the whole hardware block */
> - regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0);
> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> + SUN4I_I2S_CTRL_GL_EN, 0);
Ditto.
> }
>
> static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
> @@ -525,6 +780,15 @@ static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
> .trigger = sun4i_i2s_trigger,
> };
>
> +static const struct snd_soc_dai_ops sun8i_i2s_dai_ops = {
> + .hw_params = sun8i_i2s_hw_params,
> + .set_fmt = sun8i_i2s_set_fmt,
> + .set_sysclk = sun4i_i2s_set_sysclk,
> + .shutdown = sun4i_i2s_shutdown,
> + .startup = sun4i_i2s_startup,
> + .trigger = sun4i_i2s_trigger,
> +};
> +
> static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
> {
> struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> @@ -572,11 +836,11 @@ static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
> }
> }
>
> +
This one looks unnecessary
> static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
> {
> switch (reg) {
> case SUN4I_I2S_FIFO_RX_REG:
> - case SUN4I_I2S_FIFO_STA_REG:
This needs to be explained
> return false;
>
> default:
> @@ -588,6 +852,7 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> {
> switch (reg) {
> case SUN4I_I2S_FIFO_RX_REG:
> + case SUN4I_I2S_FIFO_STA_REG:
Ditto.
> case SUN4I_I2S_INT_STA_REG:
> case SUN4I_I2S_RX_CNT_REG:
> case SUN4I_I2S_TX_CNT_REG:
> @@ -598,6 +863,34 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> }
> }
>
> +static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case SUN8I_I2S_FIFO_TX_REG:
> + return false;
> +
> + default:
> + return true;
> + }
> +}
It also applies to the old generation.
> +static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case SUN4I_I2S_FIFO_RX_REG:
> + case SUN4I_I2S_FIFO_CTRL_REG:
> + case SUN4I_I2S_FIFO_STA_REG:
> + case SUN8I_I2S_INT_STA_REG:
> + case SUN4I_I2S_RX_CNT_REG:
> + case SUN4I_I2S_TX_CNT_REG:
> + case SUN4I_I2S_TX_CHAN_MAP_REG:
> + return true;
> +
> + default:
> + return false;
> + }
> +}
Besides the H3 interrupt status, it looks duplicated. Can't we share
both implementations with a condition for that register?
> +
> static const struct reg_default sun4i_i2s_reg_defaults[] = {
> { SUN4I_I2S_CTRL_REG, 0x00000000 },
> { SUN4I_I2S_FMT0_REG, 0x0000000c },
> @@ -611,6 +904,20 @@ static const struct reg_default sun4i_i2s_reg_defaults[] = {
> { SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
> };
>
> +static const struct reg_default sun8i_i2s_reg_defaults[] = {
> + { SUN4I_I2S_CTRL_REG, 0x00060000 },
> + { SUN4I_I2S_FMT0_REG, 0x00000033 },
> + { SUN4I_I2S_FMT1_REG, 0x00000030 },
> + { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
> + { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
> + { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
> + { SUN8I_I2S_TX_CHAN_CFG_REG, 0x00000000 },
> + { SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
> + { SUN8I_I2S_TX_CHAN_MAP_REG, 0x00000000 },
> + { SUN8I_I2S_RX_CHAN_SEL_REG, 0x00000000 },
> + { SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
> +};
> +
> static const struct regmap_config sun4i_i2s_regmap_config = {
> .reg_bits = 32,
> .reg_stride = 4,
> @@ -625,6 +932,19 @@ static const struct regmap_config sun4i_i2s_regmap_config = {
> .volatile_reg = sun4i_i2s_volatile_reg,
> };
>
> +static const struct regmap_config sun8i_i2s_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN8I_I2S_RX_CHAN_MAP_REG,
> + .cache_type = REGCACHE_FLAT,
> + .reg_defaults = sun8i_i2s_reg_defaults,
> + .num_reg_defaults = ARRAY_SIZE(sun8i_i2s_reg_defaults),
> + .writeable_reg = sun4i_i2s_wr_reg,
> + .readable_reg = sun8i_i2s_rd_reg,
> + .volatile_reg = sun8i_i2s_volatile_reg,
> +};
> +
> static int sun4i_i2s_runtime_resume(struct device *dev)
> {
> struct sun4i_i2s *i2s = dev_get_drvdata(dev);
> @@ -684,6 +1004,13 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
> .ops = &sun4i_i2s_dai_ops,
> };
>
> +static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
> + .has_reset = true,
> + .reg_offset_txdata = SUN8I_I2S_FIFO_TX_REG,
> + .sun4i_i2s_regmap = &sun8i_i2s_regmap_config,
> + .ops = &sun8i_i2s_dai_ops,
> +};
> +
> static int sun4i_i2s_probe(struct platform_device *pdev)
> {
> struct sun4i_i2s *i2s;
> @@ -823,6 +1150,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
> .compatible = "allwinner,sun6i-a31-i2s",
> .data = &sun6i_a31_i2s_quirks,
> },
> + {
> + .compatible = "allwinner,sun8i-h3-i2s",
> + .data = &sun8i_h3_i2s_quirks,
> + },
> {}
> };
> MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
> --
> 2.13.2
>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[toc] | [prev] | [next] | [standalone]
| From | Code Kipper <codekipper@gmail.com> |
|---|---|
| Date | 2017-07-05 19:50 +0200 |
| Subject | Re: [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3 |
| Message-ID | <tZWB4-Ve-17@gated-at.bofh.it> |
| In reply to | #1681639 |
On 5 July 2017 at 18:20, Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> On Wed, Jul 05, 2017 at 05:43:24PM +0200, codekipper@gmail.com wrote:
>> From: Marcus Cooper <codekipper@gmail.com>
>>
>> There are a lot of changes to the sun8i-h3 i2s block but not enough
>> to warrant to a new driver.
>>
>> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
>> ---
>> .../devicetree/bindings/sound/sun4i-i2s.txt | 2 +
>> sound/soc/sunxi/sun4i-i2s.c | 339 ++++++++++++++++++++-
>> 2 files changed, 337 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> index ee21da865771..fc5da6080759 100644
>> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> @@ -8,6 +8,7 @@ Required properties:
>> - compatible: should be one of the following:
>> - "allwinner,sun4i-a10-i2s"
>> - "allwinner,sun6i-a31-i2s"
>> + - "allwinner,sun8i-h3-i2s"
>> - reg: physical base address of the controller and length of memory mapped
>> region.
>> - interrupts: should contain the I2S interrupt.
>> @@ -22,6 +23,7 @@ Required properties:
>>
>> Required properties for the following compatibles:
>> - "allwinner,sun6i-a31-i2s"
>> + - "allwinner,sun8i-h3-i2s"
>> - resets: phandle to the reset line for this codec
>>
>> Example:
>> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
>> index bb7affd53002..0b853fe320e0 100644
>> --- a/sound/soc/sunxi/sun4i-i2s.c
>> +++ b/sound/soc/sunxi/sun4i-i2s.c
>> @@ -26,9 +26,16 @@
>> #include <sound/soc-dai.h>
>>
>> #define SUN4I_I2S_CTRL_REG 0x00
>> +#define SUN4I_I2S_CTRL_BCLK_OUT BIT(18)
>> +#define SUN4I_I2S_CTRL_LRCK_OUT BIT(17)
>> +#define SUN4I_I2S_CTRL_LRCKR_OUT BIT(16)
>> #define SUN4I_I2S_CTRL_SDO_EN_MASK GENMASK(11, 8)
>> #define SUN4I_I2S_CTRL_SDO_EN(sdo) BIT(8 + (sdo))
>> #define SUN4I_I2S_CTRL_MODE_MASK BIT(5)
>> +#define SUN8I_I2S_CTRL_MODE_MASK GENMASK(5, 4)
>> +#define SUN8I_I2S_CTRL_MODE_RIGHT_J (2 << 4)
>> +#define SUN8I_I2S_CTRL_MODE_I2S (1 << 4)
>> +#define SUN8I_I2S_CTRL_MODE_PCM (0 << 4)
>> #define SUN4I_I2S_CTRL_MODE_SLAVE (1 << 5)
>> #define SUN4I_I2S_CTRL_MODE_MASTER (0 << 5)
>> #define SUN4I_I2S_CTRL_TX_EN BIT(2)
>> @@ -36,16 +43,27 @@
>> #define SUN4I_I2S_CTRL_GL_EN BIT(0)
>>
>> #define SUN4I_I2S_FMT0_REG 0x04
>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK BIT(19)
>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED (1 << 19)
>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL (0 << 19)
>> +#define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK GENMASK(17, 8)
>> +#define SUN8I_I2S_FMT0_LRCK_PERIOD(period) ((period) << 8)
>> #define SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK BIT(7)
>> #define SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED (1 << 7)
>> #define SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL (0 << 7)
>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_MASK BIT(7)
>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 7)
>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 7)
>> #define SUN4I_I2S_FMT0_BCLK_POLARITY_MASK BIT(6)
>> #define SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 6)
>> #define SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 6)
>> #define SUN4I_I2S_FMT0_SR_MASK GENMASK(5, 4)
>> +#define SUN8I_I2S_FMT0_SR_MASK GENMASK(6, 4)
>> #define SUN4I_I2S_FMT0_SR(sr) ((sr) << 4)
>> #define SUN4I_I2S_FMT0_WSS_MASK GENMASK(3, 2)
>> #define SUN4I_I2S_FMT0_WSS(wss) ((wss) << 2)
>> +#define SUN8I_I2S_FMT0_WSS_MASK GENMASK(2, 0)
>> +#define SUN8I_I2S_FMT0_WSS(wss) (wss)
>> #define SUN4I_I2S_FMT0_FMT_MASK GENMASK(1, 0)
>> #define SUN4I_I2S_FMT0_FMT_RIGHT_J (2 << 0)
>> #define SUN4I_I2S_FMT0_FMT_LEFT_J (1 << 0)
>> @@ -53,6 +71,7 @@
>>
>> #define SUN4I_I2S_FMT1_REG 0x08
>> #define SUN4I_I2S_FIFO_TX_REG 0x0c
>> +#define SUN8I_I2S_INT_STA_REG 0x0c
>> #define SUN4I_I2S_FIFO_RX_REG 0x10
>>
>> #define SUN4I_I2S_FIFO_CTRL_REG 0x14
>> @@ -70,10 +89,13 @@
>> #define SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN BIT(3)
>>
>> #define SUN4I_I2S_INT_STA_REG 0x20
>> +#define SUN8I_I2S_FIFO_TX_REG 0x20
>>
>> #define SUN4I_I2S_CLK_DIV_REG 0x24
>> +#define SUN8I_I2S_CLK_DIV_MCLK_EN BIT(8)
>> #define SUN4I_I2S_CLK_DIV_MCLK_EN BIT(7)
>> #define SUN4I_I2S_CLK_DIV_BCLK_MASK GENMASK(6, 4)
>> +#define SUN8I_I2S_CLK_DIV_BCLK_MASK GENMASK(7, 4)
>> #define SUN4I_I2S_CLK_DIV_BCLK(bclk) ((bclk) << 4)
>> #define SUN4I_I2S_CLK_DIV_MCLK_MASK GENMASK(3, 0)
>> #define SUN4I_I2S_CLK_DIV_MCLK(mclk) ((mclk) << 0)
>> @@ -82,14 +104,27 @@
>> #define SUN4I_I2S_TX_CNT_REG 0x2c
>>
>> #define SUN4I_I2S_TX_CHAN_SEL_REG 0x30
>> +#define SUN8I_I2S_TX_CHAN_CFG_REG 0x30
>> #define SUN4I_I2S_TX_CHAN_SEL(num_chan) (((num_chan) - 1) << 0)
>>
>> #define SUN4I_I2S_TX_CHAN_MAP_REG 0x34
>> #define SUN4I_I2S_TX_CHAN_MAP(chan, sample) ((sample) << (chan << 2))
>> +#define SUN8I_I2S_TX_CHAN_SEL_REG 0x34
>> +#define SUN8I_I2S_TX_CHAN_OFFSET_MASK GENMASK(13, 12)
>> +#define SUN8I_I2S_TX_CHAN_OFFSET(offset) (offset << 12)
>> +#define SUN8I_I2S_TX_CHAN_EN_MASK GENMASK(11, 4)
>> +#define SUN8I_I2S_TX_CHAN_EN(num_chan) (((1 << num_chan) - 1) << 4)
>> +#define SUN8I_I2S_TX_CHAN_SEL_MASK GENMASK(2, 0)
>> +#define SUN8I_I2S_TX_CHAN_SEL(num_chan) (((num_chan) - 1) << 0)
>>
>> #define SUN4I_I2S_RX_CHAN_SEL_REG 0x38
>> #define SUN4I_I2S_RX_CHAN_MAP_REG 0x3c
>>
>> +#define SUN8I_I2S_TX_CHAN_MAP_REG 0x44
>> +
>> +#define SUN8I_I2S_RX_CHAN_SEL_REG 0x54
>> +#define SUN8I_I2S_RX_CHAN_MAP_REG 0x58
>> +
>> struct sun4i_i2s {
>> struct clk *bus_clk;
>> struct clk *mod_clk;
>> @@ -363,6 +398,225 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>> return 0;
>> }
>>
>> +static int sun8i_i2s_set_clk_rate(struct sun4i_i2s *i2s,
>> + unsigned int rate,
>> + unsigned int word_size)
>> +{
>> + unsigned int clk_rate;
>> + int bclk_div, mclk_div;
>> + int ret, i;
>> +
>> + switch (rate) {
>> + case 176400:
>> + case 88200:
>> + case 44100:
>> + case 22050:
>> + case 11025:
>> + clk_rate = 22579200;
>> + break;
>> +
>> + case 192000:
>> + case 128000:
>> + case 96000:
>> + case 64000:
>> + case 48000:
>> + case 32000:
>> + case 24000:
>> + case 16000:
>> + case 12000:
>> + case 8000:
>> + clk_rate = 24576000;
>> + break;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + ret = clk_set_rate(i2s->mod_clk, clk_rate);
>> + if (ret)
>> + return ret;
>> +
>> + /* Always favor the highest oversampling rate */
>> + for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) {
>> + unsigned int oversample_rate = sun4i_i2s_oversample_rates[i];
>> +
>> + bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
>> + word_size);
>> + mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
>> + clk_rate,
>> + rate);
>> +
>> + if ((bclk_div >= 0) && (mclk_div >= 0))
>> + break;
>> + }
>> +
>> + if ((bclk_div < 0) || (mclk_div < 0))
>> + return -EINVAL;
>> +
>> + regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG,
>> + SUN4I_I2S_CLK_DIV_BCLK(bclk_div) |
>> + SUN4I_I2S_CLK_DIV_MCLK(mclk_div + 1) |
>> + SUN8I_I2S_CLK_DIV_MCLK_EN);
>
> Duplicating all that code just for a single bit difference doesn't
> seem very wise. You can handle that bit difference through
> regmap_fields.
Thanks Maxime for the quick review...I'll ack all these and get back
to you if I get stuck. Just had a quick look at reusing the original
function of the above and with some additional quirks it seems to
work. Wasn't aware of the regmap_fields so I will need to investiagte
their usage, do you know of any good examples?
BR,
CK
>
>
>> + /* Set sync period */
>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>> + SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
>> + SUN8I_I2S_FMT0_LRCK_PERIOD((2 * word_size)-1));
>> + regmap_write(i2s->regmap, SUN4I_I2S_FMT1_REG, 0);
>
> It seems to be applicable for the old flavour too, why isn't it there?
>
>> +
>> + return 0;
>> +}
>> +
>> +static int sun8i_i2s_hw_params(struct snd_pcm_substream *substream,
>> + struct snd_pcm_hw_params *params,
>> + struct snd_soc_dai *dai)
>> +{
>> + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> + int sr, wss;
>> + u32 width, channels = 0;
>> +
>> + switch (params_channels(params)) {
>> + case 2:
>> + channels |= SUN4I_I2S_CTRL_SDO_EN(0);
>> + break;
>> + default:
>> + dev_err(dai->dev, "invalid channel: %d\n",
>> + params_channels(params));
>> + return -EINVAL;
>> + }
>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> + SUN4I_I2S_CTRL_SDO_EN_MASK, channels);
>
> This seems applicable to the old generation.
>
>> + /* Configure the channels */
>> + regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_CFG_REG,
>> + SUN4I_I2S_TX_CHAN_SEL(params_channels(params)));
>
> This can be handled by regmap_fields.
>
>> + /* Select the channels */
>> + regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
>> + SUN8I_I2S_TX_CHAN_EN_MASK |
>> + SUN8I_I2S_TX_CHAN_SEL_MASK,
>> + SUN8I_I2S_TX_CHAN_EN(params_channels(params)) |
>> + SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
>
> Ditto.
>
>>
>> + /* Map the channels for stereo playback */
>> + regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG,
>> + SUN4I_I2S_TX_CHAN_MAP(1, 1));
>
> Ditto.
>
>> + /* Select the channels */
>> + regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
>> + SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
>
> Ditto.
>
>> + /* Map the channels for stereo capture */
>> + regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG,
>> + SUN4I_I2S_TX_CHAN_MAP(1, 1));
>
> Ditto.
>
>> + switch (params_physical_width(params)) {
>> + case 16:
>> + width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + break;
>> + case 24:
>> + case 32:
>> + width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + i2s->playback_dma_data.addr_width = width;
>
> This is applicable to the old generation.
>
>> +
>> + switch (params_width(params)) {
>> + case 16:
>> + sr = 3;
>> + wss = 3;
>> + break;
>> + case 20:
>> + sr = 4;
>> + wss = 4;
>> + break;
>> + case 24:
>> + sr = 5;
>> + wss = 5;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>
> It looks like this changed, maybe we can split it into a separate
> function?
>
>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>> + SUN8I_I2S_FMT0_WSS_MASK | SUN8I_I2S_FMT0_SR_MASK,
>> + SUN8I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
>> +
>> + return sun8i_i2s_set_clk_rate(i2s, params_rate(params),
>> + params_width(params));
>> +}
>> +
>> +static int sun8i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>> +{
>> + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> + u32 val = SUN8I_I2S_CTRL_MODE_I2S;
>> + u32 offset = 0;
>> +
>> + /* DAI Mode */
>> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>> + case SND_SOC_DAIFMT_I2S:
>> + offset = 1;
>> + break;
>> + case SND_SOC_DAIFMT_LEFT_J:
>> + break;
>> + case SND_SOC_DAIFMT_RIGHT_J:
>> + val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> + SUN8I_I2S_CTRL_MODE_MASK,
>> + val);
>> +
>> + /* blck offset determines whether i2s or LJ */
>> + regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
>> + SUN8I_I2S_TX_CHAN_OFFSET_MASK,
>> + SUN8I_I2S_TX_CHAN_OFFSET(offset));
>> +
>> + /* DAI clock polarity */
>> + switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
>> + case SND_SOC_DAIFMT_IB_IF:
>> + /* Invert both clocks */
>> + val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
>> + SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
>> + break;
>> + case SND_SOC_DAIFMT_IB_NF:
>> + /* Invert bit clock */
>> + val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
>> + SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
>> + break;
>> + case SND_SOC_DAIFMT_NB_IF:
>> + /* Invert frame clock */
>> + val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
>> + SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;
>> + break;
>> + case SND_SOC_DAIFMT_NB_NF:
>> + /* Nothing to do for both normal cases */
>> + val = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL |
>> + SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>> + SUN8I_I2S_FMT0_BCLK_POLARITY_MASK |
>> + SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK,
>> + val);
>> +
>> + /* Set significant bits in our FIFOs */
>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
>> + SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
>> + SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
>> + SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
>> + SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
>> + return 0;
>> +}
>> +
>> static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
>> {
>> /* Flush RX FIFO */
>> @@ -471,8 +725,8 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
>> int ret = 0;
>>
>> /* Enable the whole hardware block */
>> - regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> - SUN4I_I2S_CTRL_GL_EN);
>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> + SUN4I_I2S_CTRL_GL_EN, SUN4I_I2S_CTRL_GL_EN);
>
> Why? This needs to be explained.
>
>>
>> /* Enable the first output line */
>> regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> @@ -500,7 +754,8 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
>> SUN4I_I2S_CTRL_SDO_EN_MASK, 0);
>>
>> /* Disable the whole hardware block */
>> - regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0);
>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>> + SUN4I_I2S_CTRL_GL_EN, 0);
>
> Ditto.
>
>> }
>>
>> static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>> @@ -525,6 +780,15 @@ static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
>> .trigger = sun4i_i2s_trigger,
>> };
>>
>> +static const struct snd_soc_dai_ops sun8i_i2s_dai_ops = {
>> + .hw_params = sun8i_i2s_hw_params,
>> + .set_fmt = sun8i_i2s_set_fmt,
>> + .set_sysclk = sun4i_i2s_set_sysclk,
>> + .shutdown = sun4i_i2s_shutdown,
>> + .startup = sun4i_i2s_startup,
>> + .trigger = sun4i_i2s_trigger,
>> +};
>> +
>> static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
>> {
>> struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> @@ -572,11 +836,11 @@ static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
>> }
>> }
>>
>> +
>
> This one looks unnecessary
>
>> static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
>> {
>> switch (reg) {
>> case SUN4I_I2S_FIFO_RX_REG:
>> - case SUN4I_I2S_FIFO_STA_REG:
>
> This needs to be explained
>
>> return false;
>>
>> default:
>> @@ -588,6 +852,7 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>> {
>> switch (reg) {
>> case SUN4I_I2S_FIFO_RX_REG:
>> + case SUN4I_I2S_FIFO_STA_REG:
>
> Ditto.
>
>> case SUN4I_I2S_INT_STA_REG:
>> case SUN4I_I2S_RX_CNT_REG:
>> case SUN4I_I2S_TX_CNT_REG:
>> @@ -598,6 +863,34 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>> }
>> }
>>
>> +static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
>> +{
>> + switch (reg) {
>> + case SUN8I_I2S_FIFO_TX_REG:
>> + return false;
>> +
>> + default:
>> + return true;
>> + }
>> +}
>
> It also applies to the old generation.
>
>> +static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>> +{
>> + switch (reg) {
>> + case SUN4I_I2S_FIFO_RX_REG:
>> + case SUN4I_I2S_FIFO_CTRL_REG:
>> + case SUN4I_I2S_FIFO_STA_REG:
>> + case SUN8I_I2S_INT_STA_REG:
>> + case SUN4I_I2S_RX_CNT_REG:
>> + case SUN4I_I2S_TX_CNT_REG:
>> + case SUN4I_I2S_TX_CHAN_MAP_REG:
>> + return true;
>> +
>> + default:
>> + return false;
>> + }
>> +}
>
> Besides the H3 interrupt status, it looks duplicated. Can't we share
> both implementations with a condition for that register?
>
>> +
>> static const struct reg_default sun4i_i2s_reg_defaults[] = {
>> { SUN4I_I2S_CTRL_REG, 0x00000000 },
>> { SUN4I_I2S_FMT0_REG, 0x0000000c },
>> @@ -611,6 +904,20 @@ static const struct reg_default sun4i_i2s_reg_defaults[] = {
>> { SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
>> };
>>
>> +static const struct reg_default sun8i_i2s_reg_defaults[] = {
>> + { SUN4I_I2S_CTRL_REG, 0x00060000 },
>> + { SUN4I_I2S_FMT0_REG, 0x00000033 },
>> + { SUN4I_I2S_FMT1_REG, 0x00000030 },
>> + { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
>> + { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
>> + { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
>> + { SUN8I_I2S_TX_CHAN_CFG_REG, 0x00000000 },
>> + { SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
>> + { SUN8I_I2S_TX_CHAN_MAP_REG, 0x00000000 },
>> + { SUN8I_I2S_RX_CHAN_SEL_REG, 0x00000000 },
>> + { SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
>> +};
>> +
>> static const struct regmap_config sun4i_i2s_regmap_config = {
>> .reg_bits = 32,
>> .reg_stride = 4,
>> @@ -625,6 +932,19 @@ static const struct regmap_config sun4i_i2s_regmap_config = {
>> .volatile_reg = sun4i_i2s_volatile_reg,
>> };
>>
>> +static const struct regmap_config sun8i_i2s_regmap_config = {
>> + .reg_bits = 32,
>> + .reg_stride = 4,
>> + .val_bits = 32,
>> + .max_register = SUN8I_I2S_RX_CHAN_MAP_REG,
>> + .cache_type = REGCACHE_FLAT,
>> + .reg_defaults = sun8i_i2s_reg_defaults,
>> + .num_reg_defaults = ARRAY_SIZE(sun8i_i2s_reg_defaults),
>> + .writeable_reg = sun4i_i2s_wr_reg,
>> + .readable_reg = sun8i_i2s_rd_reg,
>> + .volatile_reg = sun8i_i2s_volatile_reg,
>> +};
>> +
>> static int sun4i_i2s_runtime_resume(struct device *dev)
>> {
>> struct sun4i_i2s *i2s = dev_get_drvdata(dev);
>> @@ -684,6 +1004,13 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
>> .ops = &sun4i_i2s_dai_ops,
>> };
>>
>> +static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
>> + .has_reset = true,
>> + .reg_offset_txdata = SUN8I_I2S_FIFO_TX_REG,
>> + .sun4i_i2s_regmap = &sun8i_i2s_regmap_config,
>> + .ops = &sun8i_i2s_dai_ops,
>> +};
>> +
>> static int sun4i_i2s_probe(struct platform_device *pdev)
>> {
>> struct sun4i_i2s *i2s;
>> @@ -823,6 +1150,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
>> .compatible = "allwinner,sun6i-a31-i2s",
>> .data = &sun6i_a31_i2s_quirks,
>> },
>> + {
>> + .compatible = "allwinner,sun8i-h3-i2s",
>> + .data = &sun8i_h3_i2s_quirks,
>> + },
>> {}
>> };
>> MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
>> --
>> 2.13.2
>>
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
[toc] | [prev] | [next] | [standalone]
| From | Chen-Yu Tsai <wens@csie.org> |
|---|---|
| Date | 2017-07-06 04:10 +0200 |
| Subject | Re: [linux-sunxi] Re: [PATCH 3/3] ASoC: sun4i-i2s: Add support for H3 |
| Message-ID | <u04oV-6iD-1@gated-at.bofh.it> |
| In reply to | #1681686 |
On Thu, Jul 6, 2017 at 1:49 AM, Code Kipper <codekipper@gmail.com> wrote:
> On 5 July 2017 at 18:20, Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>> On Wed, Jul 05, 2017 at 05:43:24PM +0200, codekipper@gmail.com wrote:
>>> From: Marcus Cooper <codekipper@gmail.com>
>>>
>>> There are a lot of changes to the sun8i-h3 i2s block but not enough
>>> to warrant to a new driver.
>>>
>>> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
>>> ---
>>> .../devicetree/bindings/sound/sun4i-i2s.txt | 2 +
>>> sound/soc/sunxi/sun4i-i2s.c | 339 ++++++++++++++++++++-
>>> 2 files changed, 337 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>>> index ee21da865771..fc5da6080759 100644
>>> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>>> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>>> @@ -8,6 +8,7 @@ Required properties:
>>> - compatible: should be one of the following:
>>> - "allwinner,sun4i-a10-i2s"
>>> - "allwinner,sun6i-a31-i2s"
>>> + - "allwinner,sun8i-h3-i2s"
>>> - reg: physical base address of the controller and length of memory mapped
>>> region.
>>> - interrupts: should contain the I2S interrupt.
>>> @@ -22,6 +23,7 @@ Required properties:
>>>
>>> Required properties for the following compatibles:
>>> - "allwinner,sun6i-a31-i2s"
>>> + - "allwinner,sun8i-h3-i2s"
>>> - resets: phandle to the reset line for this codec
>>>
>>> Example:
>>> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
>>> index bb7affd53002..0b853fe320e0 100644
>>> --- a/sound/soc/sunxi/sun4i-i2s.c
>>> +++ b/sound/soc/sunxi/sun4i-i2s.c
>>> @@ -26,9 +26,16 @@
>>> #include <sound/soc-dai.h>
>>>
>>> #define SUN4I_I2S_CTRL_REG 0x00
>>> +#define SUN4I_I2S_CTRL_BCLK_OUT BIT(18)
>>> +#define SUN4I_I2S_CTRL_LRCK_OUT BIT(17)
>>> +#define SUN4I_I2S_CTRL_LRCKR_OUT BIT(16)
>>> #define SUN4I_I2S_CTRL_SDO_EN_MASK GENMASK(11, 8)
>>> #define SUN4I_I2S_CTRL_SDO_EN(sdo) BIT(8 + (sdo))
>>> #define SUN4I_I2S_CTRL_MODE_MASK BIT(5)
>>> +#define SUN8I_I2S_CTRL_MODE_MASK GENMASK(5, 4)
>>> +#define SUN8I_I2S_CTRL_MODE_RIGHT_J (2 << 4)
>>> +#define SUN8I_I2S_CTRL_MODE_I2S (1 << 4)
>>> +#define SUN8I_I2S_CTRL_MODE_PCM (0 << 4)
>>> #define SUN4I_I2S_CTRL_MODE_SLAVE (1 << 5)
>>> #define SUN4I_I2S_CTRL_MODE_MASTER (0 << 5)
>>> #define SUN4I_I2S_CTRL_TX_EN BIT(2)
>>> @@ -36,16 +43,27 @@
>>> #define SUN4I_I2S_CTRL_GL_EN BIT(0)
>>>
>>> #define SUN4I_I2S_FMT0_REG 0x04
>>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK BIT(19)
>>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED (1 << 19)
>>> +#define SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL (0 << 19)
>>> +#define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK GENMASK(17, 8)
>>> +#define SUN8I_I2S_FMT0_LRCK_PERIOD(period) ((period) << 8)
>>> #define SUN4I_I2S_FMT0_LRCLK_POLARITY_MASK BIT(7)
>>> #define SUN4I_I2S_FMT0_LRCLK_POLARITY_INVERTED (1 << 7)
>>> #define SUN4I_I2S_FMT0_LRCLK_POLARITY_NORMAL (0 << 7)
>>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_MASK BIT(7)
>>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 7)
>>> +#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 7)
>>> #define SUN4I_I2S_FMT0_BCLK_POLARITY_MASK BIT(6)
>>> #define SUN4I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 6)
>>> #define SUN4I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 6)
>>> #define SUN4I_I2S_FMT0_SR_MASK GENMASK(5, 4)
>>> +#define SUN8I_I2S_FMT0_SR_MASK GENMASK(6, 4)
>>> #define SUN4I_I2S_FMT0_SR(sr) ((sr) << 4)
>>> #define SUN4I_I2S_FMT0_WSS_MASK GENMASK(3, 2)
>>> #define SUN4I_I2S_FMT0_WSS(wss) ((wss) << 2)
>>> +#define SUN8I_I2S_FMT0_WSS_MASK GENMASK(2, 0)
>>> +#define SUN8I_I2S_FMT0_WSS(wss) (wss)
>>> #define SUN4I_I2S_FMT0_FMT_MASK GENMASK(1, 0)
>>> #define SUN4I_I2S_FMT0_FMT_RIGHT_J (2 << 0)
>>> #define SUN4I_I2S_FMT0_FMT_LEFT_J (1 << 0)
>>> @@ -53,6 +71,7 @@
>>>
>>> #define SUN4I_I2S_FMT1_REG 0x08
>>> #define SUN4I_I2S_FIFO_TX_REG 0x0c
>>> +#define SUN8I_I2S_INT_STA_REG 0x0c
>>> #define SUN4I_I2S_FIFO_RX_REG 0x10
>>>
>>> #define SUN4I_I2S_FIFO_CTRL_REG 0x14
>>> @@ -70,10 +89,13 @@
>>> #define SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN BIT(3)
>>>
>>> #define SUN4I_I2S_INT_STA_REG 0x20
>>> +#define SUN8I_I2S_FIFO_TX_REG 0x20
>>>
>>> #define SUN4I_I2S_CLK_DIV_REG 0x24
>>> +#define SUN8I_I2S_CLK_DIV_MCLK_EN BIT(8)
>>> #define SUN4I_I2S_CLK_DIV_MCLK_EN BIT(7)
>>> #define SUN4I_I2S_CLK_DIV_BCLK_MASK GENMASK(6, 4)
>>> +#define SUN8I_I2S_CLK_DIV_BCLK_MASK GENMASK(7, 4)
>>> #define SUN4I_I2S_CLK_DIV_BCLK(bclk) ((bclk) << 4)
>>> #define SUN4I_I2S_CLK_DIV_MCLK_MASK GENMASK(3, 0)
>>> #define SUN4I_I2S_CLK_DIV_MCLK(mclk) ((mclk) << 0)
>>> @@ -82,14 +104,27 @@
>>> #define SUN4I_I2S_TX_CNT_REG 0x2c
>>>
>>> #define SUN4I_I2S_TX_CHAN_SEL_REG 0x30
>>> +#define SUN8I_I2S_TX_CHAN_CFG_REG 0x30
>>> #define SUN4I_I2S_TX_CHAN_SEL(num_chan) (((num_chan) - 1) << 0)
>>>
>>> #define SUN4I_I2S_TX_CHAN_MAP_REG 0x34
>>> #define SUN4I_I2S_TX_CHAN_MAP(chan, sample) ((sample) << (chan << 2))
>>> +#define SUN8I_I2S_TX_CHAN_SEL_REG 0x34
>>> +#define SUN8I_I2S_TX_CHAN_OFFSET_MASK GENMASK(13, 12)
>>> +#define SUN8I_I2S_TX_CHAN_OFFSET(offset) (offset << 12)
>>> +#define SUN8I_I2S_TX_CHAN_EN_MASK GENMASK(11, 4)
>>> +#define SUN8I_I2S_TX_CHAN_EN(num_chan) (((1 << num_chan) - 1) << 4)
>>> +#define SUN8I_I2S_TX_CHAN_SEL_MASK GENMASK(2, 0)
>>> +#define SUN8I_I2S_TX_CHAN_SEL(num_chan) (((num_chan) - 1) << 0)
>>>
>>> #define SUN4I_I2S_RX_CHAN_SEL_REG 0x38
>>> #define SUN4I_I2S_RX_CHAN_MAP_REG 0x3c
>>>
>>> +#define SUN8I_I2S_TX_CHAN_MAP_REG 0x44
>>> +
>>> +#define SUN8I_I2S_RX_CHAN_SEL_REG 0x54
>>> +#define SUN8I_I2S_RX_CHAN_MAP_REG 0x58
>>> +
>>> struct sun4i_i2s {
>>> struct clk *bus_clk;
>>> struct clk *mod_clk;
>>> @@ -363,6 +398,225 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>>> return 0;
>>> }
>>>
>>> +static int sun8i_i2s_set_clk_rate(struct sun4i_i2s *i2s,
>>> + unsigned int rate,
>>> + unsigned int word_size)
>>> +{
>>> + unsigned int clk_rate;
>>> + int bclk_div, mclk_div;
>>> + int ret, i;
>>> +
>>> + switch (rate) {
>>> + case 176400:
>>> + case 88200:
>>> + case 44100:
>>> + case 22050:
>>> + case 11025:
>>> + clk_rate = 22579200;
>>> + break;
>>> +
>>> + case 192000:
>>> + case 128000:
>>> + case 96000:
>>> + case 64000:
>>> + case 48000:
>>> + case 32000:
>>> + case 24000:
>>> + case 16000:
>>> + case 12000:
>>> + case 8000:
>>> + clk_rate = 24576000;
>>> + break;
>>> +
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = clk_set_rate(i2s->mod_clk, clk_rate);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Always favor the highest oversampling rate */
>>> + for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) {
>>> + unsigned int oversample_rate = sun4i_i2s_oversample_rates[i];
>>> +
>>> + bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
>>> + word_size);
>>> + mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
>>> + clk_rate,
>>> + rate);
>>> +
>>> + if ((bclk_div >= 0) && (mclk_div >= 0))
>>> + break;
>>> + }
>>> +
>>> + if ((bclk_div < 0) || (mclk_div < 0))
>>> + return -EINVAL;
>>> +
>>> + regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG,
>>> + SUN4I_I2S_CLK_DIV_BCLK(bclk_div) |
>>> + SUN4I_I2S_CLK_DIV_MCLK(mclk_div + 1) |
>>> + SUN8I_I2S_CLK_DIV_MCLK_EN);
>>
>> Duplicating all that code just for a single bit difference doesn't
>> seem very wise. You can handle that bit difference through
>> regmap_fields.
>
> Thanks Maxime for the quick review...I'll ack all these and get back
> to you if I get stuck. Just had a quick look at reusing the original
> function of the above and with some additional quirks it seems to
> work. Wasn't aware of the regmap_fields so I will need to investiagte
> their usage, do you know of any good examples?
> BR,
> CK
See https://wens.tw/hdmi-i2c-regmap.txt for an example of what I'm
doing for the HDMI driver. Note that this is a bit extreme as some
regmap_fields are only 1 bit wide. This example uses regmap_fields
for all register bitfields that are different.
In other cases where only the register offset differs I would just
have regmap_field point to the whole register.
ChenYu
>>
>>
>>> + /* Set sync period */
>>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>>> + SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
>>> + SUN8I_I2S_FMT0_LRCK_PERIOD((2 * word_size)-1));
>>> + regmap_write(i2s->regmap, SUN4I_I2S_FMT1_REG, 0);
>>
>> It seems to be applicable for the old flavour too, why isn't it there?
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun8i_i2s_hw_params(struct snd_pcm_substream *substream,
>>> + struct snd_pcm_hw_params *params,
>>> + struct snd_soc_dai *dai)
>>> +{
>>> + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>>> + int sr, wss;
>>> + u32 width, channels = 0;
>>> +
>>> + switch (params_channels(params)) {
>>> + case 2:
>>> + channels |= SUN4I_I2S_CTRL_SDO_EN(0);
>>> + break;
>>> + default:
>>> + dev_err(dai->dev, "invalid channel: %d\n",
>>> + params_channels(params));
>>> + return -EINVAL;
>>> + }
>>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> + SUN4I_I2S_CTRL_SDO_EN_MASK, channels);
>>
>> This seems applicable to the old generation.
>>
>>> + /* Configure the channels */
>>> + regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_CFG_REG,
>>> + SUN4I_I2S_TX_CHAN_SEL(params_channels(params)));
>>
>> This can be handled by regmap_fields.
>>
>>> + /* Select the channels */
>>> + regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
>>> + SUN8I_I2S_TX_CHAN_EN_MASK |
>>> + SUN8I_I2S_TX_CHAN_SEL_MASK,
>>> + SUN8I_I2S_TX_CHAN_EN(params_channels(params)) |
>>> + SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
>>
>> Ditto.
>>
>>>
>>> + /* Map the channels for stereo playback */
>>> + regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG,
>>> + SUN4I_I2S_TX_CHAN_MAP(1, 1));
>>
>> Ditto.
>>
>>> + /* Select the channels */
>>> + regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
>>> + SUN8I_I2S_TX_CHAN_SEL(params_channels(params)));
>>
>> Ditto.
>>
>>> + /* Map the channels for stereo capture */
>>> + regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG,
>>> + SUN4I_I2S_TX_CHAN_MAP(1, 1));
>>
>> Ditto.
>>
>>> + switch (params_physical_width(params)) {
>>> + case 16:
>>> + width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>>> + break;
>>> + case 24:
>>> + case 32:
>>> + width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>>> + break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> + i2s->playback_dma_data.addr_width = width;
>>
>> This is applicable to the old generation.
>>
>>> +
>>> + switch (params_width(params)) {
>>> + case 16:
>>> + sr = 3;
>>> + wss = 3;
>>> + break;
>>> + case 20:
>>> + sr = 4;
>>> + wss = 4;
>>> + break;
>>> + case 24:
>>> + sr = 5;
>>> + wss = 5;
>>> + break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>
>> It looks like this changed, maybe we can split it into a separate
>> function?
>>
>>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>>> + SUN8I_I2S_FMT0_WSS_MASK | SUN8I_I2S_FMT0_SR_MASK,
>>> + SUN8I_I2S_FMT0_WSS(wss) | SUN4I_I2S_FMT0_SR(sr));
>>> +
>>> + return sun8i_i2s_set_clk_rate(i2s, params_rate(params),
>>> + params_width(params));
>>> +}
>>> +
>>> +static int sun8i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>>> +{
>>> + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>>> + u32 val = SUN8I_I2S_CTRL_MODE_I2S;
>>> + u32 offset = 0;
>>> +
>>> + /* DAI Mode */
>>> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>> + case SND_SOC_DAIFMT_I2S:
>>> + offset = 1;
>>> + break;
>>> + case SND_SOC_DAIFMT_LEFT_J:
>>> + break;
>>> + case SND_SOC_DAIFMT_RIGHT_J:
>>> + val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
>>> + break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +
>>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> + SUN8I_I2S_CTRL_MODE_MASK,
>>> + val);
>>> +
>>> + /* blck offset determines whether i2s or LJ */
>>> + regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
>>> + SUN8I_I2S_TX_CHAN_OFFSET_MASK,
>>> + SUN8I_I2S_TX_CHAN_OFFSET(offset));
>>> +
>>> + /* DAI clock polarity */
>>> + switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
>>> + case SND_SOC_DAIFMT_IB_IF:
>>> + /* Invert both clocks */
>>> + val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
>>> + SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
>>> + break;
>>> + case SND_SOC_DAIFMT_IB_NF:
>>> + /* Invert bit clock */
>>> + val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
>>> + SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
>>> + break;
>>> + case SND_SOC_DAIFMT_NB_IF:
>>> + /* Invert frame clock */
>>> + val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED |
>>> + SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;
>>> + break;
>>> + case SND_SOC_DAIFMT_NB_NF:
>>> + /* Nothing to do for both normal cases */
>>> + val = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL |
>>> + SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL;
>>> + break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +
>>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
>>> + SUN8I_I2S_FMT0_BCLK_POLARITY_MASK |
>>> + SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK,
>>> + val);
>>> +
>>> + /* Set significant bits in our FIFOs */
>>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
>>> + SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
>>> + SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
>>> + SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
>>> + SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
>>> + return 0;
>>> +}
>>> +
>>> static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
>>> {
>>> /* Flush RX FIFO */
>>> @@ -471,8 +725,8 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
>>> int ret = 0;
>>>
>>> /* Enable the whole hardware block */
>>> - regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> - SUN4I_I2S_CTRL_GL_EN);
>>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> + SUN4I_I2S_CTRL_GL_EN, SUN4I_I2S_CTRL_GL_EN);
>>
>> Why? This needs to be explained.
>>
>>>
>>> /* Enable the first output line */
>>> regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> @@ -500,7 +754,8 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream,
>>> SUN4I_I2S_CTRL_SDO_EN_MASK, 0);
>>>
>>> /* Disable the whole hardware block */
>>> - regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0);
>>> + regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>>> + SUN4I_I2S_CTRL_GL_EN, 0);
>>
>> Ditto.
>>
>>> }
>>>
>>> static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>>> @@ -525,6 +780,15 @@ static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
>>> .trigger = sun4i_i2s_trigger,
>>> };
>>>
>>> +static const struct snd_soc_dai_ops sun8i_i2s_dai_ops = {
>>> + .hw_params = sun8i_i2s_hw_params,
>>> + .set_fmt = sun8i_i2s_set_fmt,
>>> + .set_sysclk = sun4i_i2s_set_sysclk,
>>> + .shutdown = sun4i_i2s_shutdown,
>>> + .startup = sun4i_i2s_startup,
>>> + .trigger = sun4i_i2s_trigger,
>>> +};
>>> +
>>> static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
>>> {
>>> struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>>> @@ -572,11 +836,11 @@ static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
>>> }
>>> }
>>>
>>> +
>>
>> This one looks unnecessary
>>
>>> static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
>>> {
>>> switch (reg) {
>>> case SUN4I_I2S_FIFO_RX_REG:
>>> - case SUN4I_I2S_FIFO_STA_REG:
>>
>> This needs to be explained
>>
>>> return false;
>>>
>>> default:
>>> @@ -588,6 +852,7 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>>> {
>>> switch (reg) {
>>> case SUN4I_I2S_FIFO_RX_REG:
>>> + case SUN4I_I2S_FIFO_STA_REG:
>>
>> Ditto.
>>
>>> case SUN4I_I2S_INT_STA_REG:
>>> case SUN4I_I2S_RX_CNT_REG:
>>> case SUN4I_I2S_TX_CNT_REG:
>>> @@ -598,6 +863,34 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>>> }
>>> }
>>>
>>> +static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
>>> +{
>>> + switch (reg) {
>>> + case SUN8I_I2S_FIFO_TX_REG:
>>> + return false;
>>> +
>>> + default:
>>> + return true;
>>> + }
>>> +}
>>
>> It also applies to the old generation.
>>
>>> +static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>>> +{
>>> + switch (reg) {
>>> + case SUN4I_I2S_FIFO_RX_REG:
>>> + case SUN4I_I2S_FIFO_CTRL_REG:
>>> + case SUN4I_I2S_FIFO_STA_REG:
>>> + case SUN8I_I2S_INT_STA_REG:
>>> + case SUN4I_I2S_RX_CNT_REG:
>>> + case SUN4I_I2S_TX_CNT_REG:
>>> + case SUN4I_I2S_TX_CHAN_MAP_REG:
>>> + return true;
>>> +
>>> + default:
>>> + return false;
>>> + }
>>> +}
>>
>> Besides the H3 interrupt status, it looks duplicated. Can't we share
>> both implementations with a condition for that register?
>>
>>> +
>>> static const struct reg_default sun4i_i2s_reg_defaults[] = {
>>> { SUN4I_I2S_CTRL_REG, 0x00000000 },
>>> { SUN4I_I2S_FMT0_REG, 0x0000000c },
>>> @@ -611,6 +904,20 @@ static const struct reg_default sun4i_i2s_reg_defaults[] = {
>>> { SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210 },
>>> };
>>>
>>> +static const struct reg_default sun8i_i2s_reg_defaults[] = {
>>> + { SUN4I_I2S_CTRL_REG, 0x00060000 },
>>> + { SUN4I_I2S_FMT0_REG, 0x00000033 },
>>> + { SUN4I_I2S_FMT1_REG, 0x00000030 },
>>> + { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
>>> + { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
>>> + { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
>>> + { SUN8I_I2S_TX_CHAN_CFG_REG, 0x00000000 },
>>> + { SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
>>> + { SUN8I_I2S_TX_CHAN_MAP_REG, 0x00000000 },
>>> + { SUN8I_I2S_RX_CHAN_SEL_REG, 0x00000000 },
>>> + { SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
>>> +};
>>> +
>>> static const struct regmap_config sun4i_i2s_regmap_config = {
>>> .reg_bits = 32,
>>> .reg_stride = 4,
>>> @@ -625,6 +932,19 @@ static const struct regmap_config sun4i_i2s_regmap_config = {
>>> .volatile_reg = sun4i_i2s_volatile_reg,
>>> };
>>>
>>> +static const struct regmap_config sun8i_i2s_regmap_config = {
>>> + .reg_bits = 32,
>>> + .reg_stride = 4,
>>> + .val_bits = 32,
>>> + .max_register = SUN8I_I2S_RX_CHAN_MAP_REG,
>>> + .cache_type = REGCACHE_FLAT,
>>> + .reg_defaults = sun8i_i2s_reg_defaults,
>>> + .num_reg_defaults = ARRAY_SIZE(sun8i_i2s_reg_defaults),
>>> + .writeable_reg = sun4i_i2s_wr_reg,
>>> + .readable_reg = sun8i_i2s_rd_reg,
>>> + .volatile_reg = sun8i_i2s_volatile_reg,
>>> +};
>>> +
>>> static int sun4i_i2s_runtime_resume(struct device *dev)
>>> {
>>> struct sun4i_i2s *i2s = dev_get_drvdata(dev);
>>> @@ -684,6 +1004,13 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
>>> .ops = &sun4i_i2s_dai_ops,
>>> };
>>>
>>> +static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
>>> + .has_reset = true,
>>> + .reg_offset_txdata = SUN8I_I2S_FIFO_TX_REG,
>>> + .sun4i_i2s_regmap = &sun8i_i2s_regmap_config,
>>> + .ops = &sun8i_i2s_dai_ops,
>>> +};
>>> +
>>> static int sun4i_i2s_probe(struct platform_device *pdev)
>>> {
>>> struct sun4i_i2s *i2s;
>>> @@ -823,6 +1150,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
>>> .compatible = "allwinner,sun6i-a31-i2s",
>>> .data = &sun6i_a31_i2s_quirks,
>>> },
>>> + {
>>> + .compatible = "allwinner,sun8i-h3-i2s",
>>> + .data = &sun8i_h3_i2s_quirks,
>>> + },
>>> {}
>>> };
>>> MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
>>> --
>>> 2.13.2
>>>
>>
>> Thanks!
>> Maxime
>>
>> --
>> Maxime Ripard, Free Electrons
>> Embedded Linux and Kernel engineering
>> http://free-electrons.com
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web