[alsa-devel] [PATCH RFC 3/3] uda1380: make driver more powersave-friendly

[Mark Brown]

On Sun, Jun 27, 2010 at 12:07:26AM +0300, Vasily Khoruzhick wrote:
> В сообщении от 26 июня 2010 23:45:12 автор Mark Brown написал:

> > This seems odd, I'd expect the cache to be being marked as clean
> > immediately after sync?

> Nope, it is not. Only i2c and clock related regs of uda1380 can be modified 
> when there's no i2s clock, i.e. mixer regs should be updated right after i2s 
> clock was enabled, so we marking mixer-related regs cache as dirty, to make 
> sure they'll be updated when possible.

This is very much non-obvious from your code - it really needs comments
explaining why the cache sync function didn't actually manage to sync
the cache.  I'd also really expect that the dirtying of the cache would
be done when the operation that invalidates it happens, not later on
when restoring the cache.  Otherwise there's a window where the cache is
flagged as valid but is not actually so which doesn't seem robust.

> > > 		uda1380_write(codec, UDA1380_PM, R02_PON_BIAS | pm);

> > Like I said previously you really should look at using DAPM here, this
> > should make the code simpler and will let you

> Hmm, uda134x driver does pretty same things as in my patch... And it seems 
> part of your sentence is lost :(

Old drivers for fairly obscure chips aren't always a good guide to best
practices for things.

> > > -	ret = uda1380_reset(codec);
> > > -	if (ret < 0) {
> > > -		dev_err(codec->dev, "Failed to issue reset\n");
> > > -		goto err_reset;
> > > -	}

> > The reason for the reset at startup is that we don't know what state the
> > device is in when Linux gets control.

> It's softreset and it's performed by writing some value to some reg. i2c xfers 
> is not possible when codec is not enabled (it is not at this point)

It needs to happen at some point.