On Fri, 10 Sep 2010 12:47:17 +0100
Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Sep 10, 2010 at 02:23:30PM +0300, Jarkko Nikula wrote:
>
> > This patch manages all the regulators and reset since it seems that register
> > sync is needed even if only analog supplies AVDD and DRVDD are disabled.
> > This was noted when the system was running with idle behavior changed and
> > IOVDD and DVDD were on.
>
> This is normal, chips normally require all the core supplies to be
> enabled to take them out of reset.
>
Some chips were programmable with digital IO supply only. I'll leave
this comment here to show that this chip has been noted to require all
of them :-)
> > @@ -151,7 +153,8 @@ static int aic3x_write(struct snd_soc_codec *codec, unsigned int reg,
> > data[1] = value & 0xff;
> >
> > aic3x_write_reg_cache(codec, data[0], data[1]);
> > - if (codec->hw_write(codec->control_data, data, 2) == 2)
> > + if (!aic3x->power ||
> > + codec->hw_write(codec->control_data, data, 2) == 2)
> > return 0;
> > else
> > return -EIO;
>
> If you were using the soc-core stuff you'd be able to use the cache_only
> flag in the CODEC to do this. It might make sense to still use the flag
> so that when someone converts the driver to use soc-cache this doesn't
> get missed.
>
Makes sense, I'll change this.
> > static int aic3x_read(struct snd_soc_codec *codec, unsigned int reg,
> > u8 *value)
> > {
> > + struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
> > *value = reg & 0xff;
> >
> > - value[0] = i2c_smbus_read_byte_data(codec->control_data, value[0]);
> > + if (aic3x->power) {
> > + value[0] = i2c_smbus_read_byte_data(codec->control_data,
> > + value[0]);
> > + aic3x_write_reg_cache(codec, reg, *value);
> > + } else {
> > + value[0] = aic3x_read_reg_cache(codec, reg);
> > + }
> >
> > - aic3x_write_reg_cache(codec, reg, *value);
> > return 0;
> > }
>
> This seems fishy - surely the read should be being satisfied from cache
> all the time if it can be, and if the register does need to be read from
> the hardware due to being volatile then it's an error to read it while
> powered off?
Good question. I didn't look at this. Actually it seems there is a need
to add some function to indicate if headset & button detection or gpio
readings are required. We cannot shutdown if those are needed.
--
Jarkko
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
