[alsa-devel] [PATCH 2/3] ASoC: support all possible sample rates in the WM8776 driver

[Timur Tabi]

Mark Brown wrote:

> This changelog doesn't correspond to reality.  The set_sysclk() function
> in the driver makes no effort to constrain the sample rates based on
> sysclk, as is normal for CODEC drivers as the system clock is frequently
> configured based on the current sample rate (at the minute the
> configured clock is used to set up the clock dividers within the CODEC
> based on sample rate). 

My understanding of the .set_sysclk() function is that one of its primary
purpose is to tell the codec what master frequency to use for its sample rate
clock divider.  Although that doesn't technically happen in the .set_sysclk
function itself, that function is typically required.  Without that function,
codecs must hard-code their mclk frequency, which therefore also locks the list
of supported sample rates.

So although I understand that the patch description may not be 100% accurate, I
think saying that it "doesn't correspond to reality" is an exaggeration.  I can
correct the description by changing a few words around.

> Trying to implement constraints based on the
> system clock is problematic and will normally decrease the usability of
> the driver in systems where the clock rates vary.

I don't think I understand that.  set_sysclk() is suppose to increase the
usability of the driver where clock rates vary because it is *the* mechanism for
telling the driver what the clock rate is.  This is how it goes:

1. The machine driver queries the platform to determine the mclk for the codec
2. The machine driver uses set_sysclk() to tell the codec driver what that
frequency is.
3. The codec driver later uses that frequency to determine which sample rates
are actually supported.

How is this problematic?  On the P1022DS, I can dynamically switch between two
mclk frequencies on the codec just by touching a gpio.

> What's actually going on here is that the driver is being cautious about
> supporting non-audio clock rates (mostly because the digital performance
> is mainly specified for audio rates) and 192kHz was omitted from the DAC
> rates.  The change itself is OK but please resubmit with a more accurate
> changelog.

The only change that I understand that you want is to clarify that the sample
rate calculation doesn't happen directly in set_sysclk().  Is there anything
else you're expecting?


-- 
Timur Tabi
Linux kernel developer at Freescale