Re: Need help fixing pop/click artifacts in an ASOC driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/17/18 1:02 PM, Mark Brown wrote:
On Mon, Dec 17, 2018 at 12:08:36PM -0600, Pierre-Louis Bossart wrote:
On 12/17/18 11:39 AM, Mark Brown wrote:
That looks a lot like the CODEC should be exporting a GPIO driver so the
machine driver doesn't actually need the regmap?  The only register
touched is _GPIO_CONTROL_1.
I am not sure what you meant by 'exporting a GPIO driver' (mostly because I
am not familiar with any GPIO framework) but indeed the local oscillator
choice is controlled by a single register accessible through regmap - and
changes to that register should only happen when the device is a specific
state to prevent click/pops.
The GPIO framework provides a fairly simple view of GPIOs - from a user
point of view it's just getting or setting the value of a line.  It
looks like the register you're controlling isn't actually controlling a
chip feature directly but rather is setting a GPIO on the chip which
controls an external clock generator.  I could be misparsing things,
though.  I did glance at the pcm512x datasheet and didn't see pins or
anything that looked like an oscillator but I could've missed something.
You are correct, there are two variants of the Hifiberry DAC+, the 'PRO' version with two oscillators on board (SoC configured as bitclock and fsync slave) and the regular without (all clocks provided by the SoC in that case). It's indeed a GPIO control of an external component, not an on-chip capability.

The machine driver should use clk_set_rate() and not directly handle regmap
or codec stuff. If it does, or if the clock framework isn't relevant here
then we can simplify all this as suggested in
https://patchwork.kernel.org/patch/10444387/. What I was trying to do with
the github update is to keep the clock framework, tie it closer with the
codec parts with a state variable that prevents wild changes without going
back to a 'safe' idle state (similar idea as PulseAudio clock changes, which
can only happen when the PCM is not opened and used).
Right, I think bringing in the clock framework more is good -
effectively all I'm suggesting is changing the control interface used to
set the clock to add an indirection through gpiolib so you don't need to
pass the raw register map around.
this looks elegant indeed, will look into it. I'll have to learn more on gpios :-)

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux