On 2019-09-18 09:42, Jon Hunter wrote:
On 17/09/2019 19:12, Ben Dooks wrote:
From: Edward Cragg edward.cragg@codethink.co.uk
Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform' driver.
Signed-off-by: Edward Cragg edward.cragg@codethink.co.uk
sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c index ac6983c6bd72..d36b4662b420 100644 --- a/sound/soc/tegra/tegra30_i2s.c +++ b/sound/soc/tegra/tegra30_i2s.c @@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd, return 0; }
+/*
- Set up TDM
- */
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
unsigned int tx_mask, unsigned int rx_mask,int slots, int slot_width)+{
- struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
- unsigned int mask = 0, val = 0;
- dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x
"
dev_dbg() please. Also I don't think it is necessary to print both the function name and 'setting TDM', the function name should be sufficient.
Yes, already sorted from previous review.
"slots: 0x%08x " "width: %d\n",Why are there extra quotes here?
No idea, I'll remove these later.
__func__, tx_mask, rx_mask, slots, slot_width)> +- /* Set up slots and tx/rx masks */
- mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;- val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
(rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);- regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
- /* Set FSYNC width */
- regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,(slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);What happens if there is only one slot? The fsync will be the width of the slot. Typically, TDM is used with DSP-A/B formats and although the driver does not appear to program the fsync width, it probably should during the tegra30_i2s_set_fmt() depending on the format used rather than here.
Hmm, should we check.
The work was done to keep as close to the original client's 2.6 kernel as possible which set the fsync field to a whole data word. We could try and just set this to say 2 here and have a much shorter frame-sync pulse.
I'll add a check for slots < 2 and set the fsync width to 2.
Thanks for the review.
Cheers Jon