On 9. 6. 2022, at 17:16, Mark Brown broonie@kernel.org wrote:
On Thu, Jun 09, 2022 at 04:09:57PM +0200, Martin Povišer wrote:
On 9. 6. 2022, at 15:33, Mark Brown broonie@kernel.org wrote:
/** Primary FE** The mclk/fs ratio at 64 for the primary frontend is important* to ensure that the headphones codec's idea of left and right* in a stereo stream over I2S fits in nicely with everyone else's.* (This is until the headphones codec's driver supports* set_tdm_slot.)** The low mclk/fs ratio precludes transmitting more than two* channels over I2S, but that's okay since there is the secondary* FE for speaker arrays anyway.*/.mclk_fs = 64,- },
This seems weird - it looks like it's confusing MCLK and the bit clock for the audio bus. These are two different clocks. Note that it's very common for devices to require a higher MCLK/fs ratio to deliver the best audio performance, 256fs is standard.
On these machines we are not producing any other clock for the codecs besides the bit clock, so I am using MCLK interchangeably for it. (It is what the sample rate is derived from after all.)
Please don't do this, you're just making everything needlessly hard to understand by using standard terminology inappropriately and there's a risk of breakage further down the line with drivers implementing the standard ops.
OK
One of the codec drivers this is to be used with (cs42l42) expects to be given the I2S bit clock with
snd_soc_dai_set_sysclk(dai, 0, mclk, SND_SOC_CLOCK_IN);
That's very, very non-standard...
I can rename mclk to bclk in all of the code to make it clearer maybe. Also the platform driver can take the bit clock value from set_bclk_ratio, instead of set_sysclk from where it takes it now. The cs42l42 driver I can patch too to accept set_bclk_ratio.
...indeed, set_bclk_ratio() is a better interface for setting the bclk ratio - the CODEC driver should really be doing that as well.
OK, adding that to my TODOs.
This shouldn't be open coded in a driver, please factor it out into the core so we've got an API for "set limit X on control Y" then call that.
There’s already snd_soc_limit_volume, but it takes a fixed control name. Can I extend it to understand patterns beginning with a wildcard, like '* Amp Gain Volume’?
Or add a new call that does that.
OK