[PATCH 0/4] ASoC: bcm2835: Additional modes and constraint updates

[PATCH 0/4] ASoC: bcm2835: Additional modes and constraint updates

[Matthias Reichl]

Main point of this series is support for justified and DSP modes and
implementation of the set_tdm_slot interface in the bcm2835 I2S driver.

I've also updated the constraints: The switch from discrete rates to
RATE_CONTINUOUS and bump of maximum rates to 384kHz has been tested
with various cards. Enforcement of full symmetry fixes issues with
split-codec setups (eg separate ADC and DAC).

This series brings the bcm2835-i2s driver in line with the downstream
rpi tree where we've carried these patches for several months.

[PATCH 1/4] ASoC: bcm2835: Add support for TDM modes

[Matthias Reichl]

bcm2835 supports arbitrary positioning of channel data within
a frame and thus is capable of supporting TDM modes. Since
the driver is limited to 2-channel operations only TDM setups
with exactly 2 active slots are supported.

Logical TDM slot numbering follows the usual convention:

For I2S-like modes, with a 50% duty-cycle frame clock,
slots 0, 2, ... are transmitted in the first half of a frame,
slots 1, 3, ... are transmitted in the second half.

For DSP modes slot numbering is ascending: 0, 1, 2, 3, ...

Channel position calculation has been refactored to use
TDM info and moved out of hw_params.

set_tdm_slot, set_bclk_ratio and hw_params now check more
strictly if the configuration is valid. Illegal configurations
like odd number of slots in I2S mode, data lengths exceeding
slot width or frame sizes larger than the hardware limit of
1024 are rejected. Also hw_params now properly checks for
errors from clk_set_rate.

Allowed PCM formats are already guarded by stream constraints,
thus the formats check in hw_params has been removed and
data_length is now retrieved via params_width().

Also standard functions like snd_soc_params_to_bclk are now
being used instead of manual calculations to make the code
more readable.

Special care has been taken to ensure that set_bclk_ratio works
as before. The bclk ratio is mapped to a 2-channel TDM config
with a slot width of half the ratio. In order to support odd ratios,
which can't be expressed via a TDM config, the ratio (frame length)
is stored and used by hw_params.

Signed-off-by: Matthias Reichl <hias-vtPv7MOkFPkAvxtiuMwx3w@public.gmane.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 242 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 190 insertions(+), 52 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba20498202e..dcacf7f83c93 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -31,6 +31,7 @@
  * General Public License for more details.
  */
 
+#include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -99,6 +100,8 @@
 #define BCM2835_I2S_CHWID(v)		(v)
 #define BCM2835_I2S_CH1(v)		((v) << 16)
 #define BCM2835_I2S_CH2(v)		(v)
+#define BCM2835_I2S_CH1_POS(v)		BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(v))
+#define BCM2835_I2S_CH2_POS(v)		BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(v))
 
 #define BCM2835_I2S_TX_PANIC(v)	((v) << 24)
 #define BCM2835_I2S_RX_PANIC(v)	((v) << 16)
@@ -110,12 +113,19 @@
 #define BCM2835_I2S_INT_RXR		BIT(1)
 #define BCM2835_I2S_INT_TXW		BIT(0)
 
+/* Frame length register is 10 bit, maximum length 1024 */
+#define BCM2835_I2S_MAX_FRAME_LENGTH	1024
+
 /* General device struct */
 struct bcm2835_i2s_dev {
 	struct device				*dev;
 	struct snd_dmaengine_dai_dma_data	dma_data[2];
 	unsigned int				fmt;
-	unsigned int				bclk_ratio;
+	unsigned int				tdm_slots;
+	unsigned int				rx_mask;
+	unsigned int				tx_mask;
+	unsigned int				slot_width;
+	unsigned int				frame_length;
 
 	struct regmap				*i2s_regmap;
 	struct clk				*clk;
@@ -225,19 +235,117 @@ static int bcm2835_i2s_set_dai_bclk_ratio(struct snd_soc_dai *dai,
 				      unsigned int ratio)
 {
 	struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
-	dev->bclk_ratio = ratio;
+
+	if (!ratio) {
+		dev->tdm_slots = 0;
+		return 0;
+	}
+
+	if (ratio > BCM2835_I2S_MAX_FRAME_LENGTH)
+		return -EINVAL;
+
+	dev->tdm_slots = 2;
+	dev->rx_mask = 0x03;
+	dev->tx_mask = 0x03;
+	dev->slot_width = ratio / 2;
+	dev->frame_length = ratio;
+
+	return 0;
+}
+
+static int bcm2835_i2s_set_dai_tdm_slot(struct snd_soc_dai *dai,
+	unsigned int tx_mask, unsigned int rx_mask,
+	int slots, int width)
+{
+	struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
+
+	if (slots) {
+		if (slots < 0 || width < 0)
+			return -EINVAL;
+
+		/* Limit masks to available slots */
+		rx_mask &= GENMASK(slots - 1, 0);
+		tx_mask &= GENMASK(slots - 1, 0);
+
+		/*
+		 * The driver is limited to 2-channel setups.
+		 * Check that exactly 2 bits are set in the masks.
+		 */
+		if (hweight_long((unsigned long) rx_mask) != 2
+		    || hweight_long((unsigned long) tx_mask) != 2)
+			return -EINVAL;
+
+		if (slots * width > BCM2835_I2S_MAX_FRAME_LENGTH)
+			return -EINVAL;
+	}
+
+	dev->tdm_slots = slots;
+
+	dev->rx_mask = rx_mask;
+	dev->tx_mask = tx_mask;
+	dev->slot_width = width;
+	dev->frame_length = slots * width;
+
 	return 0;
 }
 
+/*
+ * Convert logical slot number into physical slot number.
+ *
+ * If odd_offset is 0 sequential number is identical to logical number.
+ * This is used for DSP modes with slot numbering 0 1 2 3 ...
+ *
+ * Otherwise odd_offset defines the physical offset for odd numbered
+ * slots. This is used for I2S and left/right justified modes to
+ * translate from logical slot numbers 0 1 2 3 ... into physical slot
+ * numbers 0 2 ... 3 4 ...
+ */
+static int bcm2835_i2s_convert_slot(unsigned int slot, unsigned int odd_offset)
+{
+	if (!odd_offset)
+		return slot;
+
+	if (slot & 1)
+		return (slot >> 1) + odd_offset;
+
+	return slot >> 1;
+}
+
+/*
+ * Calculate channel position from mask and slot width.
+ *
+ * Mask must contain exactly 2 set bits.
+ * Lowest set bit is channel 1 position, highest set bit channel 2.
+ * The constant offset is added to both channel positions.
+ *
+ * If odd_offset is > 0 slot positions are translated to
+ * I2S-style TDM slot numbering ( 0 2 ... 3 4 ...) with odd
+ * logical slot numbers starting at physical slot odd_offset.
+ */
+static void bcm2835_i2s_calc_channel_pos(
+	unsigned int *ch1_pos, unsigned int *ch2_pos,
+	unsigned int mask, unsigned int width,
+	unsigned int bit_offset, unsigned int odd_offset)
+{
+	*ch1_pos = bcm2835_i2s_convert_slot((ffs(mask) - 1), odd_offset)
+			* width + bit_offset;
+	*ch2_pos = bcm2835_i2s_convert_slot((fls(mask) - 1), odd_offset)
+			* width + bit_offset;
+}
+
 static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 				 struct snd_pcm_hw_params *params,
 				 struct snd_soc_dai *dai)
 {
 	struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
-	unsigned int sampling_rate = params_rate(params);
-	unsigned int data_length, data_delay, bclk_ratio;
-	unsigned int ch1pos, ch2pos, mode, format;
+	unsigned int data_length, data_delay, framesync_length;
+	unsigned int slots, slot_width, odd_slot_offset;
+	int frame_length, bclk_rate;
+	unsigned int rx_mask, tx_mask;
+	unsigned int rx_ch1_pos, rx_ch2_pos, tx_ch1_pos, tx_ch2_pos;
+	unsigned int mode, format;
 	uint32_t csreg;
+	int ret = 0;
 
 	/*
 	 * If a stream is already enabled,
@@ -248,39 +356,44 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	if (csreg & (BCM2835_I2S_TXON | BCM2835_I2S_RXON))
 		return 0;
 
-	/*
-	 * Adjust the data length according to the format.
-	 * We prefill the half frame length with an integer
-	 * divider of 2400 as explained at the clock settings.
-	 * Maybe it is overwritten there, if the Integer mode
-	 * does not apply.
-	 */
-	switch (params_format(params)) {
-	case SNDRV_PCM_FORMAT_S16_LE:
-		data_length = 16;
-		break;
-	case SNDRV_PCM_FORMAT_S24_LE:
-		data_length = 24;
-		break;
-	case SNDRV_PCM_FORMAT_S32_LE:
-		data_length = 32;
-		break;
-	default:
-		return -EINVAL;
+	data_length = params_width(params);
+	data_delay = 0;
+	odd_slot_offset = 0;
+	mode = 0;
+
+	if (dev->tdm_slots) {
+		slots = dev->tdm_slots;
+		slot_width = dev->slot_width;
+		frame_length = dev->frame_length;
+		rx_mask = dev->rx_mask;
+		tx_mask = dev->tx_mask;
+		bclk_rate = dev->frame_length * params_rate(params);
+	} else {
+		slots = 2;
+		slot_width = params_width(params);
+		rx_mask = 0x03;
+		tx_mask = 0x03;
+
+		frame_length = snd_soc_params_to_frame_size(params);
+		if (frame_length < 0)
+			return frame_length;
+
+		bclk_rate = snd_soc_params_to_bclk(params);
+		if (bclk_rate < 0)
+			return bclk_rate;
 	}
 
-	/* If bclk_ratio already set, use that one. */
-	if (dev->bclk_ratio)
-		bclk_ratio = dev->bclk_ratio;
-	else
-		/* otherwise calculate a fitting block ratio */
-		bclk_ratio = 2 * data_length;
+	/* Check if data fits into slots */
+	if (data_length > slot_width)
+		return -EINVAL;
 
 	/* Clock should only be set up here if CPU is clock master */
 	switch (dev->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
 	case SND_SOC_DAIFMT_CBS_CFM:
-		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
+		ret = clk_set_rate(dev->clk, bclk_rate);
+		if (ret)
+			return ret;
 		break;
 	default:
 		break;
@@ -294,9 +407,26 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	format |= BCM2835_I2S_CHWID((data_length-8)&0xf);
 
+	/* CH2 format is the same as for CH1 */
+	format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
+
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+		/* I2S mode needs an even number of slots */
+		if (slots & 1)
+			return -EINVAL;
+
+		/*
+		 * Use I2S-style logical slot numbering: even slots
+		 * are in first half of frame, odd slots in second half.
+		 */
+		odd_slot_offset = slots >> 1;
+
+		/* MSB starts one cycle after frame start */
 		data_delay = 1;
+
+		/* Setup frame sync signal for 50% duty cycle */
+		framesync_length = frame_length / 2;
 		break;
 	default:
 		/*
@@ -307,18 +437,10 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	ch1pos = data_delay;
-	ch2pos = bclk_ratio / 2 + data_delay;
-
-	switch (params_channels(params)) {
-	case 2:
-		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
-		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
-		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
-		break;
-	default:
-		return -EINVAL;
-	}
+	bcm2835_i2s_calc_channel_pos(&rx_ch1_pos, &rx_ch2_pos,
+		rx_mask, slot_width, data_delay, odd_slot_offset);
+	bcm2835_i2s_calc_channel_pos(&tx_ch1_pos, &tx_ch2_pos,
+		tx_mask, slot_width, data_delay, odd_slot_offset);
 
 	/*
 	 * Set format for both streams.
@@ -326,11 +448,16 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	 * (and therefore word length) anyway,
 	 * so the format will be the same.
 	 */
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG, format);
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG, format);
+	regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG, 
+		  format
+		| BCM2835_I2S_CH1_POS(rx_ch1_pos)
+		| BCM2835_I2S_CH2_POS(rx_ch2_pos));
+	regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG, 
+		  format
+		| BCM2835_I2S_CH1_POS(tx_ch1_pos)
+		| BCM2835_I2S_CH2_POS(tx_ch2_pos));
 
 	/* Setup the I2S mode */
-	mode = 0;
 
 	if (data_length <= 16) {
 		/*
@@ -342,8 +469,8 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		mode |= BCM2835_I2S_FTXP | BCM2835_I2S_FRXP;
 	}
 
-	mode |= BCM2835_I2S_FLEN(bclk_ratio - 1);
-	mode |= BCM2835_I2S_FSLEN(bclk_ratio / 2);
+	mode |= BCM2835_I2S_FLEN(frame_length - 1);
+	mode |= BCM2835_I2S_FSLEN(framesync_length);
 
 	/* Master or slave? */
 	switch (dev->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
@@ -423,7 +550,20 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	/* Clear FIFOs */
 	bcm2835_i2s_clear_fifos(dev, true, true);
 
-	return 0;
+	dev_dbg(dev->dev,
+		"slots: %d width: %d rx mask: 0x%02x tx_mask: 0x%02x\n",
+		slots, slot_width, rx_mask, tx_mask);
+
+	dev_dbg(dev->dev, "frame len: %d sync len: %d data len: %d\n",
+		frame_length, framesync_length, data_length);
+
+	dev_dbg(dev->dev, "rx pos: %d,%d tx pos: %d,%d\n",
+		rx_ch1_pos, rx_ch2_pos, tx_ch1_pos, tx_ch2_pos);
+
+	dev_dbg(dev->dev, "sampling rate: %d bclk rate: %d\n",
+		params_rate(params), bclk_rate);
+
+	return ret;
 }
 
 static int bcm2835_i2s_prepare(struct snd_pcm_substream *substream,
@@ -559,6 +699,7 @@ static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
 	.hw_params	= bcm2835_i2s_hw_params,
 	.set_fmt	= bcm2835_i2s_set_dai_fmt,
 	.set_bclk_ratio	= bcm2835_i2s_set_dai_bclk_ratio,
+	.set_tdm_slot	= bcm2835_i2s_set_dai_tdm_slot,
 };
 
 static int bcm2835_i2s_dai_probe(struct snd_soc_dai *dai)
@@ -699,9 +840,6 @@ static int bcm2835_i2s_probe(struct platform_device *pdev)
 	dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].flags =
 		SND_DMAENGINE_PCM_DAI_FLAG_PACK;
 
-	/* BCLK ratio - use default */
-	dev->bclk_ratio = 0;
-
 	/* Store the pdev */
 	dev->dev = &pdev->dev;
 	dev_set_drvdata(&pdev->dev, dev);
-- 
2.11.0

[PATCH 2/4] ASoC: bcm2835: Support left/right justified and DSP modes

[Matthias Reichl]

DSP modes and left/right justified modes can be supported
on bcm2835 by configuring the frame sync polarity and
frame sync length registers and by adjusting the
channel data position registers.

Clock and frame sync polarity handling in hw_params has
been refactored to make the interaction between logical
rising/falling edge frame start and physical configuration
(changed by normal/inverted polarity modes) clearer.

Modes where the first active data bit is transmitted immediately
after frame start (eg DSP mode B with slot 0 active)
only work reliable if bcm2835 is configured as frame master.
In frame slave mode channel swap (or shift, this isn't quite
clear yet) can occur.

Currently the driver only warns if an unstable configuration
is detected but doensn't prevent using them.

Signed-off-by: Matthias Reichl <hias@horus.com>
---
 sound/soc/bcm/bcm2835-i2s.c | 152 +++++++++++++++++++++++++++++---------------
 1 file changed, 99 insertions(+), 53 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index dcacf7f83c93..3a706fda4f39 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -344,6 +344,9 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	unsigned int rx_mask, tx_mask;
 	unsigned int rx_ch1_pos, rx_ch2_pos, tx_ch1_pos, tx_ch2_pos;
 	unsigned int mode, format;
+	bool bit_clock_master = false;
+	bool frame_sync_master = false;
+	bool frame_start_falling_edge = false;
 	uint32_t csreg;
 	int ret = 0;
 
@@ -387,16 +390,39 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	if (data_length > slot_width)
 		return -EINVAL;
 
-	/* Clock should only be set up here if CPU is clock master */
+	/* Check if CPU is bit clock master */
 	switch (dev->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
 	case SND_SOC_DAIFMT_CBS_CFM:
-		ret = clk_set_rate(dev->clk, bclk_rate);
-		if (ret)
-			return ret;
+		bit_clock_master = true;
+		break;
+	case SND_SOC_DAIFMT_CBM_CFS:
+	case SND_SOC_DAIFMT_CBM_CFM:
+		bit_clock_master = false;
 		break;
 	default:
+		return -EINVAL;
+	}
+
+	/* Check if CPU is frame sync master */
+	switch (dev->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFS:
+	case SND_SOC_DAIFMT_CBM_CFS:
+		frame_sync_master = true;
+		break;
+	case SND_SOC_DAIFMT_CBS_CFM:
+	case SND_SOC_DAIFMT_CBM_CFM:
+		frame_sync_master = false;
 		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Clock should only be set up here if CPU is clock master */
+	if (bit_clock_master) {
+		ret = clk_set_rate(dev->clk, bclk_rate);
+		if (ret)
+			return ret;
 	}
 
 	/* Setup the frame format */
@@ -427,13 +453,41 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 		/* Setup frame sync signal for 50% duty cycle */
 		framesync_length = frame_length / 2;
+		frame_start_falling_edge = true;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		if (slots & 1)
+			return -EINVAL;
+
+		odd_slot_offset = slots >> 1;
+		data_delay = 0;
+		framesync_length = frame_length / 2;
+		frame_start_falling_edge = false;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		if (slots & 1)
+			return -EINVAL;
+
+		/* Odd frame lengths aren't supported */
+		if (frame_length & 1)
+			return -EINVAL;
+
+		odd_slot_offset = slots >> 1;
+		data_delay = slot_width - data_length;
+		framesync_length = frame_length / 2;
+		frame_start_falling_edge = false;
+		break;
+	case SND_SOC_DAIFMT_DSP_A:
+		data_delay = 1;
+		framesync_length = 1;
+		frame_start_falling_edge = false;
+		break;
+	case SND_SOC_DAIFMT_DSP_B:
+		data_delay = 0;
+		framesync_length = 1;
+		frame_start_falling_edge = false;
 		break;
 	default:
-		/*
-		 * TODO
-		 * Others are possible but are not implemented at the moment.
-		 */
-		dev_err(dev->dev, "%s:bad format\n", __func__);
 		return -EINVAL;
 	}
 
@@ -443,6 +497,15 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		tx_mask, slot_width, data_delay, odd_slot_offset);
 
 	/*
+	 * Transmitting data immediately after frame start, eg
+	 * in left-justified or DSP mode A, only works stable
+	 * if bcm2835 is the frame clock master.
+	 */
+	if ((!rx_ch1_pos || !tx_ch1_pos) && !frame_sync_master)
+		dev_warn(dev->dev,
+			"Unstable slave config detected, L/R may be swapped");
+
+	/*
 	 * Set format for both streams.
 	 * We cannot set another frame length
 	 * (and therefore word length) anyway,
@@ -472,62 +535,38 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	mode |= BCM2835_I2S_FLEN(frame_length - 1);
 	mode |= BCM2835_I2S_FSLEN(framesync_length);
 
-	/* Master or slave? */
-	switch (dev->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
-	case SND_SOC_DAIFMT_CBS_CFS:
-		/* CPU is master */
-		break;
-	case SND_SOC_DAIFMT_CBM_CFS:
-		/*
-		 * CODEC is bit clock master
-		 * CPU is frame master
-		 */
+	/* CLKM selects bcm2835 clock slave mode */
+	if (!bit_clock_master)
 		mode |= BCM2835_I2S_CLKM;
-		break;
-	case SND_SOC_DAIFMT_CBS_CFM:
-		/*
-		 * CODEC is frame master
-		 * CPU is bit clock master
-		 */
+
+	/* FSM selects bcm2835 frame sync slave mode */
+	if (!frame_sync_master)
 		mode |= BCM2835_I2S_FSM;
+
+	/* CLKI selects normal clocking mode, sampling on rising edge */
+        switch (dev->fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+	case SND_SOC_DAIFMT_NB_IF:
+		mode |= BCM2835_I2S_CLKI;
 		break;
-	case SND_SOC_DAIFMT_CBM_CFM:
-		/* CODEC is master */
-		mode |= BCM2835_I2S_CLKM;
-		mode |= BCM2835_I2S_FSM;
+	case SND_SOC_DAIFMT_IB_NF:
+	case SND_SOC_DAIFMT_IB_IF:
 		break;
 	default:
-		dev_err(dev->dev, "%s:bad master\n", __func__);
 		return -EINVAL;
 	}
 
-	/*
-	 * Invert clocks?
-	 *
-	 * The BCM approach seems to be inverted to the classical I2S approach.
-	 */
+	/* FSI selects frame start on falling edge */
 	switch (dev->fmt & SND_SOC_DAIFMT_INV_MASK) {
 	case SND_SOC_DAIFMT_NB_NF:
-		/* None. Therefore, both for BCM */
-		mode |= BCM2835_I2S_CLKI;
-		mode |= BCM2835_I2S_FSI;
-		break;
-	case SND_SOC_DAIFMT_IB_IF:
-		/* Both. Therefore, none for BCM */
+	case SND_SOC_DAIFMT_IB_NF:
+		if (frame_start_falling_edge)
+			mode |= BCM2835_I2S_FSI;
 		break;
 	case SND_SOC_DAIFMT_NB_IF:
-		/*
-		 * Invert only frame sync. Therefore,
-		 * invert only bit clock for BCM
-		 */
-		mode |= BCM2835_I2S_CLKI;
-		break;
-	case SND_SOC_DAIFMT_IB_NF:
-		/*
-		 * Invert only bit clock. Therefore,
-		 * invert only frame sync for BCM
-		 */
-		mode |= BCM2835_I2S_FSI;
+	case SND_SOC_DAIFMT_IB_IF:
+		if (!frame_start_falling_edge)
+			mode |= BCM2835_I2S_FSI;
 		break;
 	default:
 		return -EINVAL;
@@ -563,6 +602,13 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	dev_dbg(dev->dev, "sampling rate: %d bclk rate: %d\n",
 		params_rate(params), bclk_rate);
 
+	dev_dbg(dev->dev, "CLKM: %d CLKI: %d FSM: %d FSI: %d frame start: %s edge\n",
+		!!(mode & BCM2835_I2S_CLKM),
+		!!(mode & BCM2835_I2S_CLKI),
+		!!(mode & BCM2835_I2S_FSM),
+		!!(mode & BCM2835_I2S_FSI),
+		(mode & BCM2835_I2S_FSI) ? "falling" : "rising");
+
 	return ret;
 }
 
-- 
2.11.0

[PATCH 3/4] ASoC: bcm2835: Support additional samplerates up to 384kHz

[Matthias Reichl]

Sample rates are only restricted by the capabilities of the
clock driver, so use SNDRV_PCM_RATE_CONTINUOUS instead of
SNDRV_PCM_RATE_8000_192000.

Tests (eg with pcm5122) have shown that bcm2835 works fine
in 384kHz/32bit stereo mode, so change the maximum allowed
rate from 192kHz to 384kHz.

Signed-off-by: Matthias Reichl <hias@horus.com>
---
 sound/soc/bcm/bcm2835-i2s.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 3a706fda4f39..43f5715a0d5d 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -765,7 +765,9 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.playback = {
 		.channels_min = 2,
 		.channels_max = 2,
-		.rates =	SNDRV_PCM_RATE_8000_192000,
+		.rates =	SNDRV_PCM_RATE_CONTINUOUS,
+		.rate_min =	8000,
+		.rate_max =	384000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
@@ -773,7 +775,9 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.capture = {
 		.channels_min = 2,
 		.channels_max = 2,
-		.rates =	SNDRV_PCM_RATE_8000_192000,
+		.rates =	SNDRV_PCM_RATE_CONTINUOUS,
+		.rate_min =	8000,
+		.rate_max =	384000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
-- 
2.11.0

[PATCH 4/4] ASoC: bcm2835: Enforce full symmetry

[Matthias Reichl]

bcm2835's configuration registers can't be changed when a stream
is running, which means asymmetric configurations aren't supported.

Channel and rate symmetry are already enforced by constraints
but samplebits had been missed.

As hw_params doesn't check for symmetry constraints by itself
and just returns success if a stream is running this led to
situations where asymmetric configurations were seeming to
succeed but of course didn't work because the hardware wasn't
configured at all.

Fix this by adding the missing samplerate symmetry constraint.

Signed-off-by: Matthias Reichl <hias-vtPv7MOkFPkAvxtiuMwx3w@public.gmane.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 43f5715a0d5d..2e449d7173fc 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -783,7 +783,8 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.ops = &bcm2835_i2s_dai_ops,
-	.symmetric_rates = 1
+	.symmetric_rates = 1,
+	.symmetric_samplebits = 1,
 };
 
 static bool bcm2835_i2s_volatile_reg(struct device *dev, unsigned int reg)
-- 
2.11.0

Re: [PATCH 3/4] ASoC: bcm2835: Support additional samplerates up to 384kHz

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 838 bytes --]

Matthias Reichl <hias@horus.com> writes:

> Sample rates are only restricted by the capabilities of the
> clock driver, so use SNDRV_PCM_RATE_CONTINUOUS instead of
> SNDRV_PCM_RATE_8000_192000.
>
> Tests (eg with pcm5122) have shown that bcm2835 works fine
> in 384kHz/32bit stereo mode, so change the maximum allowed
> rate from 192kHz to 384kHz.

The docs I've got say the maximum PCM clock freq is 25Mhz, so if my math
is right then 384kHz should be fine.  This patch is:

Reviewed-by: Eric Anholt <eric@anholt.net>

(There's a note about needing to be careful that the mash filter doesn't
give you frequencies over 25mhz, but we should be doing limits like that
in the clock driver)

I'm going to give the alsa folks a chance to look at the modes patches
first, since I know they've got strong opinions about them and I don't
at all.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Applied "ASoC: bcm2835: Enforce full symmetry" to the asoc tree

[Mark Brown]

The patch

   ASoC: bcm2835: Enforce full symmetry

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 3d2b3c707b3f9516d6c183eb1ffbf02a6dc5dc98 Mon Sep 17 00:00:00 2001
From: Matthias Reichl <hias@horus.com>
Date: Wed, 8 Nov 2017 21:03:32 +0100
Subject: [PATCH] ASoC: bcm2835: Enforce full symmetry

bcm2835's configuration registers can't be changed when a stream
is running, which means asymmetric configurations aren't supported.

Channel and rate symmetry are already enforced by constraints
but samplebits had been missed.

As hw_params doesn't check for symmetry constraints by itself
and just returns success if a stream is running this led to
situations where asymmetric configurations were seeming to
succeed but of course didn't work because the hardware wasn't
configured at all.

Fix this by adding the missing samplerate symmetry constraint.

Signed-off-by: Matthias Reichl <hias@horus.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index f4b778ed350b..111b4ef7ce35 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -737,7 +737,8 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.ops = &bcm2835_i2s_dai_ops,
-	.symmetric_rates = 1
+	.symmetric_rates = 1,
+	.symmetric_samplebits = 1,
 };
 
 static bool bcm2835_i2s_volatile_reg(struct device *dev, unsigned int reg)
-- 
2.15.0

Applied "ASoC: bcm2835: Add support for TDM modes" to the asoc tree

[Mark Brown]

The patch

   ASoC: bcm2835: Add support for TDM modes

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 9448572d98f358de9078be67a2a52d467e28fbd2 Mon Sep 17 00:00:00 2001
From: Matthias Reichl <hias@horus.com>
Date: Wed, 8 Nov 2017 21:03:29 +0100
Subject: [PATCH] ASoC: bcm2835: Add support for TDM modes

bcm2835 supports arbitrary positioning of channel data within
a frame and thus is capable of supporting TDM modes. Since
the driver is limited to 2-channel operations only TDM setups
with exactly 2 active slots are supported.

Logical TDM slot numbering follows the usual convention:

For I2S-like modes, with a 50% duty-cycle frame clock,
slots 0, 2, ... are transmitted in the first half of a frame,
slots 1, 3, ... are transmitted in the second half.

For DSP modes slot numbering is ascending: 0, 1, 2, 3, ...

Channel position calculation has been refactored to use
TDM info and moved out of hw_params.

set_tdm_slot, set_bclk_ratio and hw_params now check more
strictly if the configuration is valid. Illegal configurations
like odd number of slots in I2S mode, data lengths exceeding
slot width or frame sizes larger than the hardware limit of
1024 are rejected. Also hw_params now properly checks for
errors from clk_set_rate.

Allowed PCM formats are already guarded by stream constraints,
thus the formats check in hw_params has been removed and
data_length is now retrieved via params_width().

Also standard functions like snd_soc_params_to_bclk are now
being used instead of manual calculations to make the code
more readable.

Special care has been taken to ensure that set_bclk_ratio works
as before. The bclk ratio is mapped to a 2-channel TDM config
with a slot width of half the ratio. In order to support odd ratios,
which can't be expressed via a TDM config, the ratio (frame length)
is stored and used by hw_params.

Signed-off-by: Matthias Reichl <hias@horus.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 242 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 190 insertions(+), 52 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba20498202e..dcacf7f83c93 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -31,6 +31,7 @@
  * General Public License for more details.
  */
 
+#include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -99,6 +100,8 @@
 #define BCM2835_I2S_CHWID(v)		(v)
 #define BCM2835_I2S_CH1(v)		((v) << 16)
 #define BCM2835_I2S_CH2(v)		(v)
+#define BCM2835_I2S_CH1_POS(v)		BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(v))
+#define BCM2835_I2S_CH2_POS(v)		BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(v))
 
 #define BCM2835_I2S_TX_PANIC(v)	((v) << 24)
 #define BCM2835_I2S_RX_PANIC(v)	((v) << 16)
@@ -110,12 +113,19 @@
 #define BCM2835_I2S_INT_RXR		BIT(1)
 #define BCM2835_I2S_INT_TXW		BIT(0)
 
+/* Frame length register is 10 bit, maximum length 1024 */
+#define BCM2835_I2S_MAX_FRAME_LENGTH	1024
+
 /* General device struct */
 struct bcm2835_i2s_dev {
 	struct device				*dev;
 	struct snd_dmaengine_dai_dma_data	dma_data[2];
 	unsigned int				fmt;
-	unsigned int				bclk_ratio;
+	unsigned int				tdm_slots;
+	unsigned int				rx_mask;
+	unsigned int				tx_mask;
+	unsigned int				slot_width;
+	unsigned int				frame_length;
 
 	struct regmap				*i2s_regmap;
 	struct clk				*clk;
@@ -225,19 +235,117 @@ static int bcm2835_i2s_set_dai_bclk_ratio(struct snd_soc_dai *dai,
 				      unsigned int ratio)
 {
 	struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
-	dev->bclk_ratio = ratio;
+
+	if (!ratio) {
+		dev->tdm_slots = 0;
+		return 0;
+	}
+
+	if (ratio > BCM2835_I2S_MAX_FRAME_LENGTH)
+		return -EINVAL;
+
+	dev->tdm_slots = 2;
+	dev->rx_mask = 0x03;
+	dev->tx_mask = 0x03;
+	dev->slot_width = ratio / 2;
+	dev->frame_length = ratio;
+
+	return 0;
+}
+
+static int bcm2835_i2s_set_dai_tdm_slot(struct snd_soc_dai *dai,
+	unsigned int tx_mask, unsigned int rx_mask,
+	int slots, int width)
+{
+	struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
+
+	if (slots) {
+		if (slots < 0 || width < 0)
+			return -EINVAL;
+
+		/* Limit masks to available slots */
+		rx_mask &= GENMASK(slots - 1, 0);
+		tx_mask &= GENMASK(slots - 1, 0);
+
+		/*
+		 * The driver is limited to 2-channel setups.
+		 * Check that exactly 2 bits are set in the masks.
+		 */
+		if (hweight_long((unsigned long) rx_mask) != 2
+		    || hweight_long((unsigned long) tx_mask) != 2)
+			return -EINVAL;
+
+		if (slots * width > BCM2835_I2S_MAX_FRAME_LENGTH)
+			return -EINVAL;
+	}
+
+	dev->tdm_slots = slots;
+
+	dev->rx_mask = rx_mask;
+	dev->tx_mask = tx_mask;
+	dev->slot_width = width;
+	dev->frame_length = slots * width;
+
 	return 0;
 }
 
+/*
+ * Convert logical slot number into physical slot number.
+ *
+ * If odd_offset is 0 sequential number is identical to logical number.
+ * This is used for DSP modes with slot numbering 0 1 2 3 ...
+ *
+ * Otherwise odd_offset defines the physical offset for odd numbered
+ * slots. This is used for I2S and left/right justified modes to
+ * translate from logical slot numbers 0 1 2 3 ... into physical slot
+ * numbers 0 2 ... 3 4 ...
+ */
+static int bcm2835_i2s_convert_slot(unsigned int slot, unsigned int odd_offset)
+{
+	if (!odd_offset)
+		return slot;
+
+	if (slot & 1)
+		return (slot >> 1) + odd_offset;
+
+	return slot >> 1;
+}
+
+/*
+ * Calculate channel position from mask and slot width.
+ *
+ * Mask must contain exactly 2 set bits.
+ * Lowest set bit is channel 1 position, highest set bit channel 2.
+ * The constant offset is added to both channel positions.
+ *
+ * If odd_offset is > 0 slot positions are translated to
+ * I2S-style TDM slot numbering ( 0 2 ... 3 4 ...) with odd
+ * logical slot numbers starting at physical slot odd_offset.
+ */
+static void bcm2835_i2s_calc_channel_pos(
+	unsigned int *ch1_pos, unsigned int *ch2_pos,
+	unsigned int mask, unsigned int width,
+	unsigned int bit_offset, unsigned int odd_offset)
+{
+	*ch1_pos = bcm2835_i2s_convert_slot((ffs(mask) - 1), odd_offset)
+			* width + bit_offset;
+	*ch2_pos = bcm2835_i2s_convert_slot((fls(mask) - 1), odd_offset)
+			* width + bit_offset;
+}
+
 static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 				 struct snd_pcm_hw_params *params,
 				 struct snd_soc_dai *dai)
 {
 	struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
-	unsigned int sampling_rate = params_rate(params);
-	unsigned int data_length, data_delay, bclk_ratio;
-	unsigned int ch1pos, ch2pos, mode, format;
+	unsigned int data_length, data_delay, framesync_length;
+	unsigned int slots, slot_width, odd_slot_offset;
+	int frame_length, bclk_rate;
+	unsigned int rx_mask, tx_mask;
+	unsigned int rx_ch1_pos, rx_ch2_pos, tx_ch1_pos, tx_ch2_pos;
+	unsigned int mode, format;
 	uint32_t csreg;
+	int ret = 0;
 
 	/*
 	 * If a stream is already enabled,
@@ -248,39 +356,44 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	if (csreg & (BCM2835_I2S_TXON | BCM2835_I2S_RXON))
 		return 0;
 
-	/*
-	 * Adjust the data length according to the format.
-	 * We prefill the half frame length with an integer
-	 * divider of 2400 as explained at the clock settings.
-	 * Maybe it is overwritten there, if the Integer mode
-	 * does not apply.
-	 */
-	switch (params_format(params)) {
-	case SNDRV_PCM_FORMAT_S16_LE:
-		data_length = 16;
-		break;
-	case SNDRV_PCM_FORMAT_S24_LE:
-		data_length = 24;
-		break;
-	case SNDRV_PCM_FORMAT_S32_LE:
-		data_length = 32;
-		break;
-	default:
-		return -EINVAL;
+	data_length = params_width(params);
+	data_delay = 0;
+	odd_slot_offset = 0;
+	mode = 0;
+
+	if (dev->tdm_slots) {
+		slots = dev->tdm_slots;
+		slot_width = dev->slot_width;
+		frame_length = dev->frame_length;
+		rx_mask = dev->rx_mask;
+		tx_mask = dev->tx_mask;
+		bclk_rate = dev->frame_length * params_rate(params);
+	} else {
+		slots = 2;
+		slot_width = params_width(params);
+		rx_mask = 0x03;
+		tx_mask = 0x03;
+
+		frame_length = snd_soc_params_to_frame_size(params);
+		if (frame_length < 0)
+			return frame_length;
+
+		bclk_rate = snd_soc_params_to_bclk(params);
+		if (bclk_rate < 0)
+			return bclk_rate;
 	}
 
-	/* If bclk_ratio already set, use that one. */
-	if (dev->bclk_ratio)
-		bclk_ratio = dev->bclk_ratio;
-	else
-		/* otherwise calculate a fitting block ratio */
-		bclk_ratio = 2 * data_length;
+	/* Check if data fits into slots */
+	if (data_length > slot_width)
+		return -EINVAL;
 
 	/* Clock should only be set up here if CPU is clock master */
 	switch (dev->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
 	case SND_SOC_DAIFMT_CBS_CFM:
-		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
+		ret = clk_set_rate(dev->clk, bclk_rate);
+		if (ret)
+			return ret;
 		break;
 	default:
 		break;
@@ -294,9 +407,26 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	format |= BCM2835_I2S_CHWID((data_length-8)&0xf);
 
+	/* CH2 format is the same as for CH1 */
+	format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
+
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+		/* I2S mode needs an even number of slots */
+		if (slots & 1)
+			return -EINVAL;
+
+		/*
+		 * Use I2S-style logical slot numbering: even slots
+		 * are in first half of frame, odd slots in second half.
+		 */
+		odd_slot_offset = slots >> 1;
+
+		/* MSB starts one cycle after frame start */
 		data_delay = 1;
+
+		/* Setup frame sync signal for 50% duty cycle */
+		framesync_length = frame_length / 2;
 		break;
 	default:
 		/*
@@ -307,18 +437,10 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	ch1pos = data_delay;
-	ch2pos = bclk_ratio / 2 + data_delay;
-
-	switch (params_channels(params)) {
-	case 2:
-		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
-		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
-		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
-		break;
-	default:
-		return -EINVAL;
-	}
+	bcm2835_i2s_calc_channel_pos(&rx_ch1_pos, &rx_ch2_pos,
+		rx_mask, slot_width, data_delay, odd_slot_offset);
+	bcm2835_i2s_calc_channel_pos(&tx_ch1_pos, &tx_ch2_pos,
+		tx_mask, slot_width, data_delay, odd_slot_offset);
 
 	/*
 	 * Set format for both streams.
@@ -326,11 +448,16 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	 * (and therefore word length) anyway,
 	 * so the format will be the same.
 	 */
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG, format);
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG, format);
+	regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG, 
+		  format
+		| BCM2835_I2S_CH1_POS(rx_ch1_pos)
+		| BCM2835_I2S_CH2_POS(rx_ch2_pos));
+	regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG, 
+		  format
+		| BCM2835_I2S_CH1_POS(tx_ch1_pos)
+		| BCM2835_I2S_CH2_POS(tx_ch2_pos));
 
 	/* Setup the I2S mode */
-	mode = 0;
 
 	if (data_length <= 16) {
 		/*
@@ -342,8 +469,8 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		mode |= BCM2835_I2S_FTXP | BCM2835_I2S_FRXP;
 	}
 
-	mode |= BCM2835_I2S_FLEN(bclk_ratio - 1);
-	mode |= BCM2835_I2S_FSLEN(bclk_ratio / 2);
+	mode |= BCM2835_I2S_FLEN(frame_length - 1);
+	mode |= BCM2835_I2S_FSLEN(framesync_length);
 
 	/* Master or slave? */
 	switch (dev->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
@@ -423,7 +550,20 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 	/* Clear FIFOs */
 	bcm2835_i2s_clear_fifos(dev, true, true);
 
-	return 0;
+	dev_dbg(dev->dev,
+		"slots: %d width: %d rx mask: 0x%02x tx_mask: 0x%02x\n",
+		slots, slot_width, rx_mask, tx_mask);
+
+	dev_dbg(dev->dev, "frame len: %d sync len: %d data len: %d\n",
+		frame_length, framesync_length, data_length);
+
+	dev_dbg(dev->dev, "rx pos: %d,%d tx pos: %d,%d\n",
+		rx_ch1_pos, rx_ch2_pos, tx_ch1_pos, tx_ch2_pos);
+
+	dev_dbg(dev->dev, "sampling rate: %d bclk rate: %d\n",
+		params_rate(params), bclk_rate);
+
+	return ret;
 }
 
 static int bcm2835_i2s_prepare(struct snd_pcm_substream *substream,
@@ -559,6 +699,7 @@ static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
 	.hw_params	= bcm2835_i2s_hw_params,
 	.set_fmt	= bcm2835_i2s_set_dai_fmt,
 	.set_bclk_ratio	= bcm2835_i2s_set_dai_bclk_ratio,
+	.set_tdm_slot	= bcm2835_i2s_set_dai_tdm_slot,
 };
 
 static int bcm2835_i2s_dai_probe(struct snd_soc_dai *dai)
@@ -699,9 +840,6 @@ static int bcm2835_i2s_probe(struct platform_device *pdev)
 	dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].flags =
 		SND_DMAENGINE_PCM_DAI_FLAG_PACK;
 
-	/* BCLK ratio - use default */
-	dev->bclk_ratio = 0;
-
 	/* Store the pdev */
 	dev->dev = &pdev->dev;
 	dev_set_drvdata(&pdev->dev, dev);
-- 
2.15.0