On Tue, Feb 26, 2019 at 09:35:47AM +0900, Kuninori Morimoto wrote: > > @@ -97,7 +98,7 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream, > > struct simple_card_data *priv = snd_soc_card_get_drvdata(rtd->card); > > struct simple_dai_props *dai_props = > > simple_priv_to_props(priv, rtd->num); > > - unsigned int mclk, mclk_fs = 0; > > + unsigned int mclk, bclk_ratio, mclk_fs = 0, bclk_slot_ratio = 0; > > int ret = 0; > > > > if (dai_props->mclk_fs) > (snip) > > + if (bclk_slot_ratio) { > > + /* FIXME do we need to care about TDM slots here ? */ > > + bclk_ratio = snd_soc_calc_frame_size(bclk_slot_ratio, > > + params_channels(params), 1); > > + > > + ret = snd_soc_dai_set_bclk_ratio(codec_dai, bclk_ratio); > > + if (ret && ret != -ENOTSUPP) > > + goto err; > > + > > + ret = snd_soc_dai_set_bclk_ratio(cpu_dai, bclk_ratio); > > + if (ret && ret != -ENOTSUPP) > > + goto err; > > + } > > Not a big deal, but "bclk_ratio" is used only here. > We can define it here This code actually requires a lot more thought - while it may solve Sven's issue, it isn't generic. So far, I have this table put together from various sources of information: bclk_ratio sample width current mcasp fslssi kirkwood 16 32 32 64 64 24 48 48 64 64 32 64 64 64 64 Let's also consider whether it should depend on the number of channels. I2S uses a WS/LRCLK signal to differentiate each channel and to demark where the MSB bit is. If userspace negotiates one channel, what happens - if WS becomes static, then there is no signal indicating where the MSB bit is in the stream, so there is no way for a receiver to synchronise. So, it is highly unlikely that bclk_ratio = channels * sample_width. If the signal continues toggling, what does it do for the inactive channel - is that just one BCLK period long or does it assume there is still the second channel? If the former, it means we could end up with bclk_ratios of 17, 25, 33 which imho is unlikely, and would mess up TDA998x's CTS measurement. What about setups where we have multiple I2S data signals (to support multi-channel audio) - if we have four channels transmitted on two data lines (as would be required by the TDA998x), that doesn't mean BCLK becomes sample_width * 4. So, I don't think the number of channels comes into the bclk_ratio calculation at all. Given the above code, it effectively means we'd always specify channels = 2 to snd_soc_calc_frame_size(). Given that it is normal to talk about I2S being clocked at "64fs", "32fs" etc, wouldn't it just be much neater to specify _this_ value in DT, rather than half that value? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel