public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: vishnu <vravulap@amd.com>
Cc: Ravulapati Vishnu vardhan rao 
	<Vishnuvardhanrao.Ravulapati@amd.com>,
	Alexander.Deucher@amd.com, djkurtz@google.com,
	Akshu.Agrawal@amd.com, Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	Vijendar Mukunda <Vijendar.Mukunda@amd.com>,
	YueHaibing <yuehaibing@huawei.com>,
	"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	"moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER
	MANAGEM..."  <alsa-devel@alsa-project.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH v9 6/6] ASoC: amd: Added ACP3x system resume and runtime pm
Date: Tue, 19 Nov 2019 16:34:16 +0300	[thread overview]
Message-ID: <20191119133416.GB30789@kadam> (raw)
In-Reply-To: <3321478e-de8f-2eb6-6e6f-6eb621b8434b@amd.com>

On Tue, Nov 19, 2019 at 06:26:17PM +0530, vishnu wrote:
> 
> 
> On 19/11/19 6:05 PM, Dan Carpenter wrote:
> > I can't apply this because I'm not CC'd on patches 2-5.
> > 
> > On Tue, Nov 19, 2019 at 05:41:16PM +0530, Ravulapati Vishnu vardhan rao wrote:
> > > +static int acp3x_power_on(void __iomem *acp3x_base)
> > > +{
> > > +	u32 val;
> > > +	u32 timeout;
> > > +
> > > +	timeout = 0;
> > > +	val = rv_readl(acp3x_base + mmACP_PGFSM_STATUS);
> > > +
> > > +	if (val == 0)
> > > +		return val;
> > > +
> > > +	if (!((val & ACP_PGFSM_STATUS_MASK) ==
> > > +				ACP_POWER_ON_IN_PROGRESS))
> > > +		rv_writel(ACP_PGFSM_CNTL_POWER_ON_MASK,
> > > +			acp3x_base + mmACP_PGFSM_CONTROL);
> > > +	while (++timeout) {
> > 
> > while (++timeout < 500)
> > 
> 
> If I check with timeout<500 and in next condition i have
> if(timeout >500) this never happens.

I was maybe not clear enough.  Please don't write:

	while (++timeout) {

That doesn't make sense as a loop.  It looks like you are trying to
loop UINT_MAX times.  Put the ++ and the limit on the same line.

There is only one real bug in my review but there is just a lot of clean
up left.  Can you have a co-worker review your patch before resending?
The patch 1/6 looks pretty good now but I haven't seen patches 2-5 so
I'm worried there is a lot of cleanup left to do.

regards,
dan carpenter


  reply	other threads:[~2019-11-19 13:37 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1574165476-24987-1-git-send-email-Vishnuvardhanrao.Ravulapati@amd.com>
2019-11-19 12:11 ` [PATCH v9 1/6] ASoC: amd:Create multiple I2S platform device endpoint Ravulapati Vishnu vardhan rao
2019-11-19 12:11 ` [RESEND PATCH v9 2/6] ASoC: amd: Refactoring of DAI from DMA driver Ravulapati Vishnu vardhan rao
2019-11-19 12:11 ` [RESEND PATCH v9 3/6] ASoC: amd: Enabling I2S instance in DMA and DAI Ravulapati Vishnu vardhan rao
2019-11-19 12:11 ` [RESEND PATCH v9 4/6] ASoC: amd: add ACP3x TDM mode support Ravulapati Vishnu vardhan rao
2019-11-19 12:11 ` [RESEND PATCH v9 5/6] ASoC: amd: Handle ACP3x I2S-SP Interrupts Ravulapati Vishnu vardhan rao
2019-11-19 12:11 ` [RESEND PATCH v9 6/6] ASoC: amd: Added ACP3x system resume and runtime pm Ravulapati Vishnu vardhan rao
2019-11-19 12:35   ` Dan Carpenter
2019-11-19 12:56     ` vishnu
2019-11-19 13:34       ` Dan Carpenter [this message]
2019-11-19 17:55         ` Mark Brown
2019-11-19 14:03       ` vishnu
2019-11-19 14:10         ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191119133416.GB30789@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=Akshu.Agrawal@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Vijendar.Mukunda@amd.com \
    --cc=Vishnuvardhanrao.Ravulapati@amd.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=djkurtz@google.com \
    --cc=gustavo@embeddedor.com \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    --cc=vravulap@amd.com \
    --cc=yuehaibing@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox