On Wed, Sep 29, 2010, Mark Brown wrote:
On Wed, Sep 29, 2010 at 02:42:32PM -0700, Peter Hsiang wrote:
On Wed, Sep 29, 2010, Mark Brown wrote:
On Tue, Sep 28, 2010 at 07:34:34PM -0700, Peter Hsiang wrote:
You should add value muxes like we have for DAPM.
Please clarify what you mean by referencing the specific code usage case in the dapm source module.
You're looking for the non-DAPM equivalent of SND_SOC_DAPM_VALUE_MUX().
This is a simple table lookup of a register value from the index number given by SOC_ENUM, the same way it's been done in other drivers.
I found a "case snd_soc_dapm_value_mux:" in dapm_set_path_status() Is this what you are referring to? How is the code there relevant to this?
/* powering down headphone gracefully */status = snd_soc_read(codec, M98088_REG_4D_PWR_EN_OUT);if ((status & M98088_HPEN) == M98088_HPEN) {max98088_hw_write(codec, M98088_REG_4D_PWR_EN_OUT,(status & ~M98088_HPEN));}schedule_timeout(msecs_to_jiffies(20));This looks rather like it should just be a post event implementing a timeout?
This needs to work as a pre event.
Again, why is this?
When powering down the headphone, the way that DAPM works is it likes to power off one item at a time, for example, the left channel, then right channel. The headphone hardware likes to see the headphone bits L and R be powered down together, for optimum result. This works best with the pre method. Powering up one channel at a time later is fine, when DAPM resumes.
case SND_SOC_BIAS_STANDBY:max98088_sync_cache(codec);snd_soc_update_bits(codec, M98088_REG_4C_PWR_EN_IN,M98088_MBEN, M98088_MBEN);break;Do you really want to sync the cache *every* time you go into standby?
The sync_cache function itself will just return if the codec->cache_sync flag is cleared from the first time it ran. You do the exact same thing in your codec driver... What is the change that you are suggesting?
The cache syncs should be part of some operation which would make it useful to sync the cache rather than just located at some point in the driver without any particular reason. For example, with the drivers I've worked on the cache is synced after we enable the supplies for the device since the CODEC may have been powered off and therefore lost any register settings that might have been done. If the cache sync is not associated with any such event then it's at best redundant and at worst the driver will loose some robustness since it becomes unclear if the events which cause a cache sync to be required are joined up with the triggering of a sync.
I see :)
+module_init(max98088_init);
Normally this would be next to the function it references.
Is this a new formatting style of the kernel now all across, or is this a personal preference?
It's a global style for the kernel, though not enforced with 100% success.
Haha, got it. I will help you with this statistic from now on.