| Message ID | 1428483128-23498-2-git-send-email-wxt@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Caesar, Am Mittwoch, 8. April 2015, 16:52:08 schrieb Caesar Wang: > To fix pop noise when shutdown,the pop noise during shutdown > is the pmic cutoff power of codec without any notice. > > Signed-off-by: jay.xu <xjq@rock-chips.com> > Signed-off-by: zhengxing <zhengxing@rock-chips.com> > Signed-off-by: Caesar Wang <wxt@rock-chips.com> > > Serien-cc: linux-kernel@vger.kernel.org > Serien-cc: devicetree@vger.kernel.org > Serien-cc: dianders@chromium.org > > --- > > sound/soc/codecs/max98090.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c > index b112b1c..066954a0 100644 > --- a/sound/soc/codecs/max98090.c > +++ b/sound/soc/codecs/max98090.c > @@ -2611,6 +2611,22 @@ static int max98090_i2c_remove(struct i2c_client > *client) return 0; > } > > +static void max98090_i2c_shutdown(struct i2c_client *i2c) > +{ > + struct max98090_priv *max98090 = dev_get_drvdata(&i2c->dev); > + > + dev_info(&i2c->dev, "shut down device\n"); is this dev_info really necessary? dev_dbg might be better, or leave it out completely, as it doesn't really provide any additional benefit. > + > + /* Enable volume smoothing, disable zero cross. This will cause > + * a quick 40ms ramp to mute on shutdown. > + */ Comment style is off ... should be /* * Enable volume smoothing, disable zero cross. This will cause * a quick 40ms ramp to mute on shutdown. */ > + regmap_write(max98090->regmap, > + M98090_REG_LEVEL_CONTROL, M98090_VSENN_MASK); > + regmap_write(max98090->regmap, > + M98090_REG_DEVICE_SHUTDOWN, 0x00); > + msleep(40); > +} > + > #ifdef CONFIG_PM > static int max98090_runtime_resume(struct device *dev) > { > @@ -2697,6 +2713,7 @@ static struct i2c_driver max98090_i2c_driver = { > }, > .probe = max98090_i2c_probe, > .remove = max98090_i2c_remove, > + .shutdown = max98090_i2c_shutdown, > .id_table = max98090_i2c_id, > };
On Wed, Apr 08, 2015 at 04:52:08PM +0800, Caesar Wang wrote: > +static void max98090_i2c_shutdown(struct i2c_client *i2c) > +{ > + struct max98090_priv *max98090 = dev_get_drvdata(&i2c->dev); > + > + dev_info(&i2c->dev, "shut down device\n"); Remove this, it's adding noise. > + > + /* Enable volume smoothing, disable zero cross. This will cause > + * a quick 40ms ramp to mute on shutdown. > + */ > + regmap_write(max98090->regmap, > + M98090_REG_LEVEL_CONTROL, M98090_VSENN_MASK); > + regmap_write(max98090->regmap, > + M98090_REG_DEVICE_SHUTDOWN, 0x00); > + msleep(40); > +} This is OK but equivalent code should be being added to the driver remove path as the same thing should be happening there.
Heiko, ? 2015?04?08? 17:25, Heiko Stübner ??: > Hi Caesar, > > Am Mittwoch, 8. April 2015, 16:52:08 schrieb Caesar Wang: >> To fix pop noise when shutdown,the pop noise during shutdown >> is the pmic cutoff power of codec without any notice. >> >> Signed-off-by: jay.xu <xjq@rock-chips.com> >> Signed-off-by: zhengxing <zhengxing@rock-chips.com> >> Signed-off-by: Caesar Wang <wxt@rock-chips.com> >> >> Serien-cc: linux-kernel@vger.kernel.org >> Serien-cc: devicetree@vger.kernel.org >> Serien-cc: dianders@chromium.org >> >> --- >> >> sound/soc/codecs/max98090.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c >> index b112b1c..066954a0 100644 >> --- a/sound/soc/codecs/max98090.c >> +++ b/sound/soc/codecs/max98090.c >> @@ -2611,6 +2611,22 @@ static int max98090_i2c_remove(struct i2c_client >> *client) return 0; >> } >> >> +static void max98090_i2c_shutdown(struct i2c_client *i2c) >> +{ >> + struct max98090_priv *max98090 = dev_get_drvdata(&i2c->dev); >> + >> + dev_info(&i2c->dev, "shut down device\n"); > is this dev_info really necessary? dev_dbg might be better, or leave it out > completely, as it doesn't really provide any additional benefit. > As Mark suggestion, I will remove it. >> + >> + /* Enable volume smoothing, disable zero cross. This will cause >> + * a quick 40ms ramp to mute on shutdown. >> + */ > Comment style is off ... should be > > /* > * Enable volume smoothing, disable zero cross. This will cause > * a quick 40ms ramp to mute on shutdown. > */ > Done >> + regmap_write(max98090->regmap, >> + M98090_REG_LEVEL_CONTROL, M98090_VSENN_MASK); >> + regmap_write(max98090->regmap, >> + M98090_REG_DEVICE_SHUTDOWN, 0x00); >> + msleep(40); >> +} >> + >> #ifdef CONFIG_PM >> static int max98090_runtime_resume(struct device *dev) >> { >> @@ -2697,6 +2713,7 @@ static struct i2c_driver max98090_i2c_driver = { >> }, >> .probe = max98090_i2c_probe, >> .remove = max98090_i2c_remove, >> + .shutdown = max98090_i2c_shutdown, >> .id_table = max98090_i2c_id, >> }; > > >
Mark, ? 2015?04?08? 17:50, Mark Brown ??: > On Wed, Apr 08, 2015 at 04:52:08PM +0800, Caesar Wang wrote: > >> +static void max98090_i2c_shutdown(struct i2c_client *i2c) >> +{ >> + struct max98090_priv *max98090 = dev_get_drvdata(&i2c->dev); >> + >> + dev_info(&i2c->dev, "shut down device\n"); > Remove this, it's adding noise. Done >> + >> + /* Enable volume smoothing, disable zero cross. This will cause >> + * a quick 40ms ramp to mute on shutdown. >> + */ >> + regmap_write(max98090->regmap, >> + M98090_REG_LEVEL_CONTROL, M98090_VSENN_MASK); >> + regmap_write(max98090->regmap, >> + M98090_REG_DEVICE_SHUTDOWN, 0x00); >> + msleep(40); >> +} > This is OK but equivalent code should be being added to the driver > remove path as the same thing should be happening there. Yeah.I will fix it in v2 patch. Thanks!
diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c index b112b1c..066954a0 100644 --- a/sound/soc/codecs/max98090.c +++ b/sound/soc/codecs/max98090.c @@ -2611,6 +2611,22 @@ static int max98090_i2c_remove(struct i2c_client *client) return 0; } +static void max98090_i2c_shutdown(struct i2c_client *i2c) +{ + struct max98090_priv *max98090 = dev_get_drvdata(&i2c->dev); + + dev_info(&i2c->dev, "shut down device\n"); + + /* Enable volume smoothing, disable zero cross. This will cause + * a quick 40ms ramp to mute on shutdown. + */ + regmap_write(max98090->regmap, + M98090_REG_LEVEL_CONTROL, M98090_VSENN_MASK); + regmap_write(max98090->regmap, + M98090_REG_DEVICE_SHUTDOWN, 0x00); + msleep(40); +} + #ifdef CONFIG_PM static int max98090_runtime_resume(struct device *dev) { @@ -2697,6 +2713,7 @@ static struct i2c_driver max98090_i2c_driver = { }, .probe = max98090_i2c_probe, .remove = max98090_i2c_remove, + .shutdown = max98090_i2c_shutdown, .id_table = max98090_i2c_id, };