[PATCH] ASoC: max98390: Remove unnecessary amp on/off conrtol

Steve Lee posted 1 patch 3 years, 7 months ago
There is a newer version of this series
sound/soc/codecs/max98390.c | 6 ------
1 file changed, 6 deletions(-)
[PATCH] ASoC: max98390: Remove unnecessary amp on/off conrtol
Posted by Steve Lee 3 years, 7 months ago
 The Amp is already control in userspace before trigger calibrate function.
Remove unnecessary control in calibrate function.

Signed-off-by: Steve Lee <steve.lee.analog@gmail.com>
---
 sound/soc/codecs/max98390.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/sound/soc/codecs/max98390.c b/sound/soc/codecs/max98390.c
index 91c0bf3d76fc..99e769190568 100644
--- a/sound/soc/codecs/max98390.c
+++ b/sound/soc/codecs/max98390.c
@@ -826,9 +826,6 @@ static int max98390_dsm_calibrate(struct snd_soc_component *component)
 	struct max98390_priv *max98390 =
 		snd_soc_component_get_drvdata(component);
 
-	regmap_write(max98390->regmap, MAX98390_R203A_AMP_EN, 0x81);
-	regmap_write(max98390->regmap, MAX98390_R23FF_GLOBAL_EN, 0x01);
-
 	regmap_read(max98390->regmap,
 		THERMAL_RDC_RD_BACK_BYTE1, &rdc);
 	regmap_read(max98390->regmap,
@@ -847,9 +844,6 @@ static int max98390_dsm_calibrate(struct snd_soc_component *component)
 	dev_info(component->dev, "rdc resistance about %d.%02d ohm, reg=0x%X temp reg=0x%X\n",
 		 rdc_integer, rdc_factor, rdc_cal_result, temp);
 
-	regmap_write(max98390->regmap, MAX98390_R23FF_GLOBAL_EN, 0x00);
-	regmap_write(max98390->regmap, MAX98390_R203A_AMP_EN, 0x80);
-
 	return 0;
 }
 
-- 
2.17.1
Re: [PATCH] ASoC: max98390: Remove unnecessary amp on/off conrtol
Posted by Mark Brown 3 years, 7 months ago
On Fri, Aug 26, 2022 at 11:35:04AM +0900, Steve Lee wrote:

>  The Amp is already control in userspace before trigger calibrate function.
> Remove unnecessary control in calibrate function.

I can't see anything which ensures that this is the case?  Should there
be a check which returns an error if the output is not enabled, or
should the function check the current state and preserve it at the end?
I can see that this would fix problems with it being disabled when
callibrating.
Re: [PATCH] ASoC: max98390: Remove unnecessary amp on/off conrtol
Posted by Lee Steve 3 years, 7 months ago
On Thu, Sep 1, 2022 at 1:36 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Aug 26, 2022 at 11:35:04AM +0900, Steve Lee wrote:
>
> >  The Amp is already control in userspace before trigger calibrate function.
> > Remove unnecessary control in calibrate function.
>
> I can't see anything which ensures that this is the case?  Should there
> be a check which returns an error if the output is not enabled, or
> should the function check the current state and preserve it at the end?
> I can see that this would fix problems with it being disabled when
> callibrating.

 As your comment, this can fix amp being disabled when calibrating.
And this also fix the case that music play right after calibration.
Actually, calibration process should start mute playback before
trigger this function.
AMP disable/enable control in calibration function can break sync with
userspace enable/disable control.
Re: [PATCH] ASoC: max98390: Remove unnecessary amp on/off conrtol
Posted by Mark Brown 3 years, 7 months ago
On Thu, Sep 01, 2022 at 02:57:19PM +0900, Lee Steve wrote:
> On Thu, Sep 1, 2022 at 1:36 AM Mark Brown <broonie@kernel.org> wrote:

> > I can't see anything which ensures that this is the case?  Should there
> > be a check which returns an error if the output is not enabled, or
> > should the function check the current state and preserve it at the end?
> > I can see that this would fix problems with it being disabled when
> > callibrating.

>  As your comment, this can fix amp being disabled when calibrating.
> And this also fix the case that music play right after calibration.
> Actually, calibration process should start mute playback before
> trigger this function.
> AMP disable/enable control in calibration function can break sync with
> userspace enable/disable control.

Right, so that sounds like there should be something which checks the
current state and doesn't start callibration unless the device is in a
sensible state.
Re: [PATCH] ASoC: max98390: Remove unnecessary amp on/off conrtol
Posted by Lee Steve 3 years, 7 months ago
On Thu, Sep 1, 2022 at 7:54 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Sep 01, 2022 at 02:57:19PM +0900, Lee Steve wrote:
> > On Thu, Sep 1, 2022 at 1:36 AM Mark Brown <broonie@kernel.org> wrote:
>
> > > I can't see anything which ensures that this is the case?  Should there
> > > be a check which returns an error if the output is not enabled, or
> > > should the function check the current state and preserve it at the end?
> > > I can see that this would fix problems with it being disabled when
> > > callibrating.
>
> >  As your comment, this can fix amp being disabled when calibrating.
> > And this also fix the case that music play right after calibration.
> > Actually, calibration process should start mute playback before
> > trigger this function.
> > AMP disable/enable control in calibration function can break sync with
> > userspace enable/disable control.
>
> Right, so that sounds like there should be something which checks the
> current state and doesn't start callibration unless the device is in a
> sensible state.

 I will send v2 patch adding check before start calibration.
Thanks.