[PATCH v2 2/2] ASoC: sun4i-i2s: Add support for H3

[maxime.ripard@free-electrons.com (Maxime Ripard)]

Hi,

On Sat, Jul 22, 2017 at 08:53:52AM +0200, codekipper at gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
> 
> The sun8i-h3 introduces a lot of changes to the i2s block such
> as different register locations, extended clock division and
> more operational modes. As we have to consider the earlier
> implementation then these changes need to be isolated.
> 
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
>  .../devicetree/bindings/sound/sun4i-i2s.txt        |   2 +
>  sound/soc/sunxi/sun4i-i2s.c                        | 202 +++++++++++++++++++++
>  2 files changed, 204 insertions(+)
> 
> 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 1854405cbcb1..2b3c2b28059c 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -26,6 +26,8 @@
>  #include <sound/soc-dai.h>
>  
>  #define SUN4I_I2S_CTRL_REG		0x00
> +#define SUN8I_I2S_CTRL_BCLK_OUT			BIT(18)
> +#define SUN8I_I2S_CTRL_LRCK_OUT			BIT(17)
>  #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)
> @@ -55,6 +57,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
> @@ -72,8 +75,10 @@
>  #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		8
>  #define SUN4I_I2S_CLK_DIV_MCLK_EN		7
>  #define SUN4I_I2S_CLK_DIV_BCLK_MASK		GENMASK(6, 4)
>  #define SUN4I_I2S_CLK_DIV_BCLK(bclk)			((bclk) << 4)
> @@ -86,16 +91,29 @@
>  #define SUN4I_I2S_TX_CHAN_SEL_REG	0x30
>  #define SUN4I_I2S_CHAN_SEL(num_chan)		(((num_chan) - 1) << 0)
>  
> +#define SUN8I_I2S_CHAN_CFG_REG		0x30
> +
>  #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(offset)	(offset << 12)
>  #define SUN4I_I2S_TX_CHAN_EN(num_chan)		(((1 << num_chan) - 1))
>  
>  #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
> +

I would not interleave those defines. It's a bit hard to see which
generation has which set of registers. I guess you could just move the
new ones to the bottom of the defines.

>  struct sun4i_i2s_quirks {
>  	bool				has_reset;
>  	bool				has_master_slave_sel;
> +	bool				has_fmt_set_lrck_period;
> +	bool				has_chcfg;
> +	bool				has_chsel_tx_chen;
> +	bool				has_chsel_offset;
>  	unsigned int			reg_offset_txdata;	/* TX FIFO */
>  	unsigned int			reg_offset_txchanmap;
>  	unsigned int			reg_offset_rxchanmap;
> @@ -113,6 +131,11 @@ struct sun4i_i2s_quirks {
>  	struct reg_field		field_fmt_set_mode;
>  	struct reg_field		field_txchansel;
>  	struct reg_field		field_rxchansel;
> +	struct reg_field		field_fmt_set_lrck_period;
> +	struct reg_field		field_chcfg_tx_slot_num;
> +	struct reg_field		field_chcfg_rx_slot_num;
> +	struct reg_field		field_chsel_tx_chen;
> +	struct reg_field		field_chsel_offset;

If you have booleans already, I guess you don't really need the
regmap_fields. You won't make that setup in the !h3 case, so the
regmap_field mapping is going to be useless anyway.

>  };
>  
>  struct sun4i_i2s {
> @@ -136,6 +159,11 @@ struct sun4i_i2s {
>  	struct regmap_field	*field_fmt_set_mode;
>  	struct regmap_field	*field_txchansel;
>  	struct regmap_field	*field_rxchansel;
> +	struct regmap_field	*field_fmt_set_lrck_period;
> +	struct regmap_field	*field_chcfg_tx_slot_num;
> +	struct regmap_field	*field_chcfg_rx_slot_num;
> +	struct regmap_field	*field_chsel_tx_chen;
> +	struct regmap_field	*field_chsel_offset;
>  
>  	const struct sun4i_i2s_quirks	*variant;
>  };
> @@ -152,6 +180,14 @@ static const struct sun4i_i2s_clk_div sun4i_i2s_bclk_div[] = {
>  	{ .div = 8, .val = 3 },
>  	{ .div = 12, .val = 4 },
>  	{ .div = 16, .val = 5 },
> +	{ .div = 24, .val = 6 },
> +	{ .div = 32, .val = 7 },
> +	{ .div = 48, .val = 8 },
> +	{ .div = 64, .val = 9 },
> +	{ .div = 96, .val = 10 },
> +	{ .div = 128, .val = 11 },
> +	{ .div = 176, .val = 12 },
> +	{ .div = 192, .val = 13 },
>  };
>  
>  static const struct sun4i_i2s_clk_div sun4i_i2s_mclk_div[] = {
> @@ -163,6 +199,13 @@ static const struct sun4i_i2s_clk_div sun4i_i2s_mclk_div[] = {
>  	{ .div = 12, .val = 5 },
>  	{ .div = 16, .val = 6 },
>  	{ .div = 24, .val = 7 },
> +	{ .div = 32, .val = 8 },
> +	{ .div = 48, .val = 9 },
> +	{ .div = 64, .val = 10 },
> +	{ .div = 96, .val = 11 },
> +	{ .div = 128, .val = 12 },
> +	{ .div = 176, .val = 13 },
> +	{ .div = 192, .val = 14 },
>  };

I'm not sure about this one. We might end up on !h3 with an invalid
divider.

>  static int sun4i_i2s_get_bclk_div(struct sun4i_i2s *i2s,
> @@ -270,6 +313,10 @@ static int sun4i_i2s_set_clk_rate(struct sun4i_i2s *i2s,
>  
>  	regmap_field_write(i2s->field_clkdiv_mclk_en, 1);
>  
> +	/* Set sync period */
> +	if (i2s->variant->has_fmt_set_lrck_period)
> +		regmap_field_write(i2s->field_fmt_set_lrck_period, 0x1f);
> +
>  	return 0;
>  }
>  
> @@ -284,6 +331,13 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
>  	if (params_channels(params) != 2)
>  		return -EINVAL;
>  
> +	if (i2s->variant->has_chcfg) {
> +		regmap_field_write(i2s->field_chcfg_tx_slot_num,
> +				   params_channels(params) - 1);
> +		regmap_field_write(i2s->field_chcfg_rx_slot_num,
> +				   params_channels(params) - 1);
> +	}
> +
>  	/* Map the channels for playback */
>  	regmap_write(i2s->regmap, i2s->variant->reg_offset_txchanmap,
>  		     0x76543210);
> @@ -299,6 +353,9 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
>  	regmap_field_write(i2s->field_rxchansel,
>  			   SUN4I_I2S_CHAN_SEL(params_channels(params)));
>  
> +	if (i2s->variant->has_chsel_tx_chen)
> +		regmap_field_write(i2s->field_chsel_tx_chen,
> +				   SUN4I_I2S_TX_CHAN_EN(params_channels(params)));
>  
>  	switch (params_physical_width(params)) {
>  	case 16:
> @@ -332,6 +389,7 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>  {
>  	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>  	u32 val;
> +	u32 offset = 0;
>  	u32 bclk_polarity = SUN4I_I2S_FMT0_POLARITY_NORMAL;
>  	u32 lrclk_polarity = SUN4I_I2S_FMT0_POLARITY_NORMAL;
>  
> @@ -339,6 +397,7 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>  	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>  	case SND_SOC_DAIFMT_I2S:
>  		val = SUN4I_I2S_FMT0_FMT_I2S;
> +		offset = 1;
>  		break;
>  	case SND_SOC_DAIFMT_LEFT_J:
>  		val = SUN4I_I2S_FMT0_FMT_LEFT_J;
> @@ -350,6 +409,19 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>  		return -EINVAL;
>  	}
>  
> +	if (i2s->variant->has_chsel_offset) {
> +		/*
> +		 * offset being set indicates that we're connected to an i2s
> +		 * device, however offset is only used on the sun8i block and
> +		 * i2s shares the same setting with the LJ format. Increment
> +		 * val so that the bit to value to write is correct.
> +		 */
> +		if (offset > 0)
> +			val++;
> +		/* blck offset determines whether i2s or LJ */
> +		regmap_field_write(i2s->field_chsel_offset, offset);
> +	}
> +
>  	regmap_field_write(i2s->field_fmt_set_mode, val);
>  
>  	/* DAI clock polarity */
> @@ -393,6 +465,29 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>  		regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
>  				   SUN4I_I2S_CTRL_MODE_MASK,
>  				   val);
> +	} else {
> +		/*
> +		 * The newer i2s block does not have a slave select bit,
> +		 * instead the clk pins are configured as inputs.
> +		 */
> +		/* DAI clock master masks */
> +		switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +		case SND_SOC_DAIFMT_CBS_CFS:
> +			/* BCLK and LRCLK master */
> +			val = SUN8I_I2S_CTRL_BCLK_OUT |
> +				SUN8I_I2S_CTRL_LRCK_OUT;
> +			break;
> +		case SND_SOC_DAIFMT_CBM_CFM:
> +			/* BCLK and LRCLK slave */
> +			val = 0;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
> +				   SUN8I_I2S_CTRL_BCLK_OUT |
> +				   SUN8I_I2S_CTRL_LRCK_OUT,
> +				   val);
>  	}
>  
>  	/* Set significant bits in our FIFOs */
> @@ -642,6 +737,15 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
>  	}
>  }
>  
> +static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +
> +	if (reg == SUN8I_I2S_INT_STA_REG)
> +		return true;
> +
> +	return sun4i_i2s_volatile_reg(dev, reg);
> +}

This means that SUN8I_I2S_FIFO_TX_REG will be treated as volatile.

Thanks!
maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170725/b70e2087/attachment-0001.sig>