linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] ASoC: max98373: Added max98373_reset for stable amp reset
@ 2018-11-28  3:20 Ryan Lee
  2018-11-28  9:49 ` Mark Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Ryan Lee @ 2018-11-28  3:20 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Ryan Lee, Grant Grundler, Kuninori Morimoto, Benson Leung,
	alsa-devel, linux-kernel
  Cc: ryan.lee.maxim

Signed-off-by: Ryan Lee <ryans.lee@maximintegrated.com>
---
Changes since v1 : Removed unusual repeat for amp software reset and verification.
                   Amp software reset will be performed once and it repeats verification maximum 3 times if it is failed.
                   Wait 10ms before every verification trial. Maximum 30ms delay will be applied to wait AMP idle state.

Changes : Created max98373_reset function to minimize code duplication.
          Changed regmap_write to regmap_update_bits. Other bits except LSB need to be masked.
          Added reset verification step to make sure software reset is completed well. Software reset is done in 10ms in normal case.
          Revision ID is available when the amp is in the idle state which means software reset is completed.
          Software reset will be performed maximum 3 times to avoid amp reset failure. Generally it is done in the first trial.
          sleep time after software reset is increased + 30ms for every retrial. Maximum possible msleep time is 100 ms (initial 10 ms + 30 ms * 3 times).

 sound/soc/codecs/max98373.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/max98373.c b/sound/soc/codecs/max98373.c
index a09d013..9c8616a 100644
--- a/sound/soc/codecs/max98373.c
+++ b/sound/soc/codecs/max98373.c
@@ -724,14 +724,39 @@ static struct snd_soc_dai_driver max98373_dai[] = {
 	}
 };
 
+static void max98373_reset(struct max98373_priv *max98373, struct device *dev)
+{
+	int ret, reg, count;
+
+	/* Software Reset */
+	ret = regmap_update_bits(max98373->regmap,
+		MAX98373_R2000_SW_RESET,
+		MAX98373_SOFT_RESET,
+		MAX98373_SOFT_RESET);
+	if (ret)
+		dev_err(dev, "Reset command failed. (ret:%d)\n", ret);
+
+	count = 0;
+	while (count < 3) {
+		usleep_range(10000, 11000);
+		/* Software Reset Verification */
+		ret = regmap_read(max98373->regmap,
+			MAX98373_R21FF_REV_ID, &reg);
+		if (!ret) {
+			dev_info(dev, "Reset completed (retry:%d)\n", count);
+			return;
+		}
+		count++;
+	}
+	dev_err(dev, "Reset failed. (ret:%d)\n", ret);
+}
+
 static int max98373_probe(struct snd_soc_component *component)
 {
 	struct max98373_priv *max98373 = snd_soc_component_get_drvdata(component);
 
 	/* Software Reset */
-	regmap_write(max98373->regmap,
-		MAX98373_R2000_SW_RESET, MAX98373_SOFT_RESET);
-	usleep_range(10000, 11000);
+	max98373_reset(max98373, component->dev);
 
 	/* IV default slot configuration */
 	regmap_write(max98373->regmap,
@@ -818,9 +843,7 @@ static int max98373_resume(struct device *dev)
 {
 	struct max98373_priv *max98373 = dev_get_drvdata(dev);
 
-	regmap_write(max98373->regmap,
-		MAX98373_R2000_SW_RESET, MAX98373_SOFT_RESET);
-	usleep_range(10000, 11000);
+	max98373_reset(max98373, dev);
 	regcache_cache_only(max98373->regmap, false);
 	regcache_sync(max98373->regmap);
 	return 0;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] ASoC: max98373: Added max98373_reset for stable amp reset
  2018-11-28  3:20 [PATCH V2] ASoC: max98373: Added max98373_reset for stable amp reset Ryan Lee
@ 2018-11-28  9:49 ` Mark Brown
  2018-11-28 17:07   ` Ryan Lee
  0 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2018-11-28  9:49 UTC (permalink / raw)
  To: Ryan Lee
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Grant Grundler,
	Kuninori Morimoto, Benson Leung, alsa-devel, linux-kernel,
	ryan.lee.maxim

[-- Attachment #1: Type: text/plain, Size: 945 bytes --]

On Wed, Nov 28, 2018 at 03:20:16AM +0000, Ryan Lee wrote:
> Signed-off-by: Ryan Lee <ryans.lee@maximintegrated.com>
> ---

Not seeing a changelog here like I asked for :(

> Changes : Created max98373_reset function to minimize code duplication.
>           Changed regmap_write to regmap_update_bits. Other bits except LSB need to be masked.
>           Added reset verification step to make sure software reset is completed well. Software reset is done in 10ms in normal case.
>           Revision ID is available when the amp is in the idle state which means software reset is completed.
>           Software reset will be performed maximum 3 times to avoid amp reset failure. Generally it is done in the first trial.
>           sleep time after software reset is increased + 30ms for every retrial. Maximum possible msleep time is 100 ms (initial 10 ms + 30 ms * 3 times).

This looks like it's supposed to be a changelog but it isn't one?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH V2] ASoC: max98373: Added max98373_reset for stable amp reset
  2018-11-28  9:49 ` Mark Brown
@ 2018-11-28 17:07   ` Ryan Lee
  2018-11-29  1:55     ` Grant Grundler
  0 siblings, 1 reply; 7+ messages in thread
From: Ryan Lee @ 2018-11-28 17:07 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Grant Grundler,
	Kuninori Morimoto, Benson Leung, alsa-devel, linux-kernel,
	ryan.lee.maxim

>-----Original Message-----
>From: Mark Brown <broonie@kernel.org>
>Sent: Wednesday, November 28, 2018 1:50 AM
>To: Ryan Lee <RyanS.Lee@maximintegrated.com>
>Cc: Liam Girdwood <lgirdwood@gmail.com>; Jaroslav Kysela
><perex@perex.cz>; Takashi Iwai <tiwai@suse.com>; Grant Grundler
><grundler@chromium.org>; Kuninori Morimoto
><kuninori.morimoto.gx@renesas.com>; Benson Leung
><bleung@chromium.org>; alsa-devel@alsa-project.org; linux-
>kernel@vger.kernel.org; ryan.lee.maxim@gmail.com
>Subject: Re: [PATCH V2] ASoC: max98373: Added max98373_reset for stable
>amp reset
>
>On Wed, Nov 28, 2018 at 03:20:16AM +0000, Ryan Lee wrote:
>> Signed-off-by: Ryan Lee <ryans.lee@maximintegrated.com>
>> ---
>
>Not seeing a changelog here like I asked for :(

Actually I added changelog as below. Do you mean this is not sufficient?

Changes since v1 : Removed unusual repeat for amp software reset and verification.
                   Amp software reset will be performed once and it repeats verification maximum 3 times if it is failed.
                   Wait 10ms before every verification trial. Maximum 30ms delay will be applied to wait AMP idle state.

>
>> Changes : Created max98373_reset function to minimize code duplication.
>>           Changed regmap_write to regmap_update_bits. Other bits except LSB
>need to be masked.
>>           Added reset verification step to make sure software reset is completed
>well. Software reset is done in 10ms in normal case.
>>           Revision ID is available when the amp is in the idle state which means
>software reset is completed.
>>           Software reset will be performed maximum 3 times to avoid amp reset
>failure. Generally it is done in the first trial.
>>           sleep time after software reset is increased + 30ms for every retrial.
>Maximum possible msleep time is 100 ms (initial 10 ms + 30 ms * 3 times).
>
>This looks like it's supposed to be a changelog but it isn't one?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] ASoC: max98373: Added max98373_reset for stable amp reset
  2018-11-28 17:07   ` Ryan Lee
@ 2018-11-29  1:55     ` Grant Grundler
  2018-11-29 11:26       ` Mark Brown
  2018-11-30  3:10       ` Ryan Lee
  0 siblings, 2 replies; 7+ messages in thread
From: Grant Grundler @ 2018-11-29  1:55 UTC (permalink / raw)
  To: RyanS.Lee
  Cc: broonie, Liam Girdwood, perex, tiwai, Grant Grundler,
	Kuninori Morimoto, Benson Leung, alsa-devel, LKML,
	ryan.lee.maxim

On Wed, Nov 28, 2018 at 9:07 AM Ryan Lee <RyanS.Lee@maximintegrated.com> wrote:
>
> >-----Original Message-----
> >From: Mark Brown <broonie@kernel.org>
> >Sent: Wednesday, November 28, 2018 1:50 AM
> >To: Ryan Lee <RyanS.Lee@maximintegrated.com>
> >Cc: Liam Girdwood <lgirdwood@gmail.com>; Jaroslav Kysela
> ><perex@perex.cz>; Takashi Iwai <tiwai@suse.com>; Grant Grundler
> ><grundler@chromium.org>; Kuninori Morimoto
> ><kuninori.morimoto.gx@renesas.com>; Benson Leung
> ><bleung@chromium.org>; alsa-devel@alsa-project.org; linux-
> >kernel@vger.kernel.org; ryan.lee.maxim@gmail.com
> >Subject: Re: [PATCH V2] ASoC: max98373: Added max98373_reset for stable
> >amp reset
> >
> >On Wed, Nov 28, 2018 at 03:20:16AM +0000, Ryan Lee wrote:
> >> Signed-off-by: Ryan Lee <ryans.lee@maximintegrated.com>
> >> ---
> >
> >Not seeing a changelog here like I asked for :(
>
> Actually I added changelog as below. Do you mean this is not sufficient?

The text is probably sufficient but not in a format that Mark can
directly apply.
Please take a quick look at Documentation/process/submitting-patches.rst.

Mark wants the "commit message" to be before the '---' line. So move
the "Changes:" text up to become the commit message and drop the
"Changes" line. That should explain why this commit is needed and
include the S-o-B line.

cheers,
grant

>
> Changes since v1 : Removed unusual repeat for amp software reset and verification.
>                    Amp software reset will be performed once and it repeats verification maximum 3 times if it is failed.
>                    Wait 10ms before every verification trial. Maximum 30ms delay will be applied to wait AMP idle state.
>
> >
> >> Changes : Created max98373_reset function to minimize code duplication.
> >>           Changed regmap_write to regmap_update_bits. Other bits except LSB
> >need to be masked.
> >>           Added reset verification step to make sure software reset is completed
> >well. Software reset is done in 10ms in normal case.
> >>           Revision ID is available when the amp is in the idle state which means
> >software reset is completed.
> >>           Software reset will be performed maximum 3 times to avoid amp reset
> >failure. Generally it is done in the first trial.
> >>           sleep time after software reset is increased + 30ms for every retrial.
> >Maximum possible msleep time is 100 ms (initial 10 ms + 30 ms * 3 times).
> >
> >This looks like it's supposed to be a changelog but it isn't one?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] ASoC: max98373: Added max98373_reset for stable amp reset
  2018-11-29  1:55     ` Grant Grundler
@ 2018-11-29 11:26       ` Mark Brown
  2018-11-30  3:17         ` Ryan Lee
  2018-11-30  3:10       ` Ryan Lee
  1 sibling, 1 reply; 7+ messages in thread
From: Mark Brown @ 2018-11-29 11:26 UTC (permalink / raw)
  To: Grant Grundler
  Cc: RyanS.Lee, Liam Girdwood, perex, tiwai, Kuninori Morimoto,
	Benson Leung, alsa-devel, LKML, ryan.lee.maxim

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

On Wed, Nov 28, 2018 at 05:55:48PM -0800, Grant Grundler wrote:
> On Wed, Nov 28, 2018 at 9:07 AM Ryan Lee <RyanS.Lee@maximintegrated.com> wrote:

> > >Not seeing a changelog here like I asked for :(

> > Actually I added changelog as below. Do you mean this is not sufficient?

> The text is probably sufficient but not in a format that Mark can
> directly apply.
> Please take a quick look at Documentation/process/submitting-patches.rst.

> Mark wants the "commit message" to be before the '---' line. So move
> the "Changes:" text up to become the commit message and drop the
> "Changes" line. That should explain why this commit is needed and
> include the S-o-B line.

Right.  If you compare what's in git and what you're sending with other
commits and mails and make sure everything looks similar you're probably
on the right track.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH V2] ASoC: max98373: Added max98373_reset for stable amp reset
  2018-11-29  1:55     ` Grant Grundler
  2018-11-29 11:26       ` Mark Brown
@ 2018-11-30  3:10       ` Ryan Lee
  1 sibling, 0 replies; 7+ messages in thread
From: Ryan Lee @ 2018-11-30  3:10 UTC (permalink / raw)
  To: Grant Grundler
  Cc: broonie, Liam Girdwood, perex, tiwai, Kuninori Morimoto,
	Benson Leung, alsa-devel, LKML, ryan.lee.maxim

>-----Original Message-----
>From: Grant Grundler <grundler@chromium.org>
>Sent: Wednesday, November 28, 2018 5:56 PM
>To: Ryan Lee <RyanS.Lee@maximintegrated.com>
>Cc: broonie@kernel.org; Liam Girdwood <lgirdwood@gmail.com>;
>perex@perex.cz; tiwai@suse.com; Grant Grundler
><grundler@chromium.org>; Kuninori Morimoto
><kuninori.morimoto.gx@renesas.com>; Benson Leung
><bleung@chromium.org>; alsa-devel@alsa-project.org; LKML <linux-
>kernel@vger.kernel.org>; ryan.lee.maxim@gmail.com
>Subject: Re: [PATCH V2] ASoC: max98373: Added max98373_reset for stable
>amp reset
>
>EXTERNAL EMAIL
>
>
>
>On Wed, Nov 28, 2018 at 9:07 AM Ryan Lee
><RyanS.Lee@maximintegrated.com> wrote:
>>
>> >-----Original Message-----
>> >From: Mark Brown <broonie@kernel.org>
>> >Sent: Wednesday, November 28, 2018 1:50 AM
>> >To: Ryan Lee <RyanS.Lee@maximintegrated.com>
>> >Cc: Liam Girdwood <lgirdwood@gmail.com>; Jaroslav Kysela
>> ><perex@perex.cz>; Takashi Iwai <tiwai@suse.com>; Grant Grundler
>> ><grundler@chromium.org>; Kuninori Morimoto
>> ><kuninori.morimoto.gx@renesas.com>; Benson Leung
>> ><bleung@chromium.org>; alsa-devel@alsa-project.org; linux-
>> >kernel@vger.kernel.org; ryan.lee.maxim@gmail.com
>> >Subject: Re: [PATCH V2] ASoC: max98373: Added max98373_reset for
>> >stable amp reset
>> >
>> >On Wed, Nov 28, 2018 at 03:20:16AM +0000, Ryan Lee wrote:
>> >> Signed-off-by: Ryan Lee <ryans.lee@maximintegrated.com>
>> >> ---
>> >
>> >Not seeing a changelog here like I asked for :(
>>
>> Actually I added changelog as below. Do you mean this is not sufficient?
>
>The text is probably sufficient but not in a format that Mark can directly apply.
>Please take a quick look at Documentation/process/submitting-patches.rst.
>
>Mark wants the "commit message" to be before the '---' line. So move the
>"Changes:" text up to become the commit message and drop the "Changes"
>line. That should explain why this commit is needed and include the S-o-B line.
Thanks for your help. I will fix it.
>
>cheers,
>grant
>
>>
>> Changes since v1 : Removed unusual repeat for amp software reset and
>verification.
>>                    Amp software reset will be performed once and it repeats
>verification maximum 3 times if it is failed.
>>                    Wait 10ms before every verification trial. Maximum 30ms delay will
>be applied to wait AMP idle state.
>>
>> >
>> >> Changes : Created max98373_reset function to minimize code duplication.
>> >>           Changed regmap_write to regmap_update_bits. Other bits
>> >> except LSB
>> >need to be masked.
>> >>           Added reset verification step to make sure software reset
>> >> is completed
>> >well. Software reset is done in 10ms in normal case.
>> >>           Revision ID is available when the amp is in the idle
>> >> state which means
>> >software reset is completed.
>> >>           Software reset will be performed maximum 3 times to avoid
>> >> amp reset
>> >failure. Generally it is done in the first trial.
>> >>           sleep time after software reset is increased + 30ms for every retrial.
>> >Maximum possible msleep time is 100 ms (initial 10 ms + 30 ms * 3 times).
>> >
>> >This looks like it's supposed to be a changelog but it isn't one?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH V2] ASoC: max98373: Added max98373_reset for stable amp reset
  2018-11-29 11:26       ` Mark Brown
@ 2018-11-30  3:17         ` Ryan Lee
  0 siblings, 0 replies; 7+ messages in thread
From: Ryan Lee @ 2018-11-30  3:17 UTC (permalink / raw)
  To: Mark Brown, Grant Grundler
  Cc: Liam Girdwood, perex, tiwai, Kuninori Morimoto, Benson Leung,
	alsa-devel, LKML, ryan.lee.maxim

>-----Original Message-----
>From: Mark Brown <broonie@kernel.org>
>Sent: Thursday, November 29, 2018 3:26 AM
>To: Grant Grundler <grundler@chromium.org>
>Cc: Ryan Lee <RyanS.Lee@maximintegrated.com>; Liam Girdwood
><lgirdwood@gmail.com>; perex@perex.cz; tiwai@suse.com; Kuninori
>Morimoto <kuninori.morimoto.gx@renesas.com>; Benson Leung
><bleung@chromium.org>; alsa-devel@alsa-project.org; LKML <linux-
>kernel@vger.kernel.org>; ryan.lee.maxim@gmail.com
>Subject: Re: [PATCH V2] ASoC: max98373: Added max98373_reset for stable
>amp reset
>
>On Wed, Nov 28, 2018 at 05:55:48PM -0800, Grant Grundler wrote:
>> On Wed, Nov 28, 2018 at 9:07 AM Ryan Lee
><RyanS.Lee@maximintegrated.com> wrote:
>
>> > >Not seeing a changelog here like I asked for :(
>
>> > Actually I added changelog as below. Do you mean this is not sufficient?
>
>> The text is probably sufficient but not in a format that Mark can
>> directly apply.
>> Please take a quick look at Documentation/process/submitting-patches.rst.
>
>> Mark wants the "commit message" to be before the '---' line. So move
>> the "Changes:" text up to become the commit message and drop the
>> "Changes" line. That should explain why this commit is needed and
>> include the S-o-B line.
>
>Right.  If you compare what's in git and what you're sending with other
>commits and mails and make sure everything looks similar you're probably on
>the right track.
I'm sorry for the mistake. Let me fix it.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-11-30  3:18 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-28  3:20 [PATCH V2] ASoC: max98373: Added max98373_reset for stable amp reset Ryan Lee
2018-11-28  9:49 ` Mark Brown
2018-11-28 17:07   ` Ryan Lee
2018-11-29  1:55     ` Grant Grundler
2018-11-29 11:26       ` Mark Brown
2018-11-30  3:17         ` Ryan Lee
2018-11-30  3:10       ` Ryan Lee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).