[alsa-devel] [PATCH 0/3] ASoC: Add I2S support for Allwinner H3 SoCs
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
From: Marcus Cooper <codekipper@gmail.com> The Allwinner H3 has some differences to its I2S block such as a different address for the TXFIFO and various register changes. This patch prepares the driver for these changes. Signed-off-by: Marcus Cooper <codekipper@gmail.com> --- sound/soc/sunxi/sun4i-i2s.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 3635bbc72cbc..38ab0144f897 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -544,7 +544,6 @@ static struct snd_soc_dai_driver sun4i_i2s_dai = { .rates = SNDRV_PCM_RATE_8000_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE, }, - .ops = &sun4i_i2s_dai_ops, .symmetric_rates = 1, }; @@ -655,15 +654,24 @@ static int sun4i_i2s_runtime_suspend(struct device *dev) } struct sun4i_i2s_quirks { - bool has_reset; + bool has_reset; + unsigned int reg_offset_txdata; /* TX FIFO */ + const struct regmap_config *sun4i_i2s_regmap; + const struct snd_soc_dai_ops *ops; }; static const struct sun4i_i2s_quirks sun4i_a10_i2s_quirks = { - .has_reset = false, + .has_reset = false, + .reg_offset_txdata = SUN4I_I2S_FIFO_TX_REG, + .sun4i_i2s_regmap = &sun4i_i2s_regmap_config, + .ops = &sun4i_i2s_dai_ops, }; static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = { - .has_reset = true, + .has_reset = true, + .reg_offset_txdata = SUN4I_I2S_FIFO_TX_REG, + .sun4i_i2s_regmap = &sun4i_i2s_regmap_config, + .ops = &sun4i_i2s_dai_ops, }; static int sun4i_i2s_probe(struct platform_device *pdev) @@ -702,8 +710,14 @@ static int sun4i_i2s_probe(struct platform_device *pdev) return PTR_ERR(i2s->bus_clk); } + quirks = of_device_get_match_data(&pdev->dev); + if (quirks == NULL) { + dev_err(&pdev->dev, "Failed to determine the quirks to use\n"); + return -ENODEV; + } + i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs, - &sun4i_i2s_regmap_config); + quirks->sun4i_i2s_regmap); if (IS_ERR(i2s->regmap)) { dev_err(&pdev->dev, "Regmap initialisation failed\n"); return PTR_ERR(i2s->regmap); @@ -732,7 +746,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev) } } - i2s->playback_dma_data.addr = res->start + SUN4I_I2S_FIFO_TX_REG; + i2s->playback_dma_data.addr = res->start + quirks->reg_offset_txdata; i2s->playback_dma_data.maxburst = 8; i2s->capture_dma_data.addr = res->start + SUN4I_I2S_FIFO_RX_REG; @@ -745,6 +759,8 @@ static int sun4i_i2s_probe(struct platform_device *pdev) goto err_pm_disable; } + /* Register ops with dai */ + sun4i_i2s_dai.ops = quirks->ops; ret = devm_snd_soc_register_component(&pdev->dev, &sun4i_i2s_component, &sun4i_i2s_dai, 1); -- 2.13.2
From: Marcus Cooper <codekipper@gmail.com> The set_fmt function pointer is called during probing and this is whilst the block is disabled. It is over writing the default register values with the same settings so isn't noticed. This wasn't a problem with the older SoCs but with the desire to reuse as much functionlity as possible for the newer devices then set_fmt needs to be called whilst the block is enabled. Signed-off-by: Marcus Cooper <codekipper@gmail.com> --- sound/soc/sunxi/sun4i-i2s.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 38ab0144f897..bb7affd53002 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -351,6 +351,15 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK, SUN4I_I2S_FIFO_CTRL_TX_MODE(1) | SUN4I_I2S_FIFO_CTRL_RX_MODE(1)); + + /* Enable the first two channels */ + regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_SEL_REG, + SUN4I_I2S_TX_CHAN_SEL(2)); + + /* Map them to the two first samples coming in */ + regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, + SUN4I_I2S_TX_CHAN_MAP(0, 0) | SUN4I_I2S_TX_CHAN_MAP(1, 1)); + return 0; } @@ -457,6 +466,9 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai); + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct device *dev = rtd->card->dev; + int ret = 0; /* Enable the whole hardware block */ regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, @@ -467,13 +479,11 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream, SUN4I_I2S_CTRL_SDO_EN_MASK, SUN4I_I2S_CTRL_SDO_EN(0)); - /* Enable the first two channels */ - regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_SEL_REG, - SUN4I_I2S_TX_CHAN_SEL(2)); - - /* Map them to the two first samples coming in */ - regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, - SUN4I_I2S_TX_CHAN_MAP(0, 0) | SUN4I_I2S_TX_CHAN_MAP(1, 1)); + ret = snd_soc_dai_set_fmt(rtd->cpu_dai, rtd->dai_link->dai_fmt); + if (ret < 0) { + dev_err(dev, "can't set cpu_dai set fmt: %d\n", ret); + return ret; + } return clk_prepare_enable(i2s->mod_clk); } -- 2.13.2
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
On Wed, Jul 05, 2017 at 05:43:23PM +0200, codekipper@gmail.com wrote:
From: Marcus Cooper <codekipper@gmail.com>
The set_fmt function pointer is called during probing and this is whilst the block is disabled. It is over writing the default register values with the same settings so isn't noticed. This wasn't a problem with the older SoCs but with the desire to reuse as much functionlity as possible for the newer devices then set_fmt needs to be called whilst the block is enabled.
The driver while the block is disabled will use the cache, and will flush it when the block is enabled so it won't make any difference. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
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
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
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.
participants (4)
-
Chen-Yu Tsai -
Code Kipper -
codekipper＠gmail.com
-
Maxime Ripard