[v2,1/6] ASoC: Intel: Add helper to poll register for DSP status
diff mbox

Message ID 1435919647-14049-2-git-send-email-vinod.koul@intel.com
State New
Headers show

Commit Message

Vinod Koul July 3, 2015, 10:34 a.m. UTC
From: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>

This patch adds helper to poll register for DSP status.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Kp, Jeeja <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/common/sst-dsp.c | 23 +++++++++++++++++++++++
 sound/soc/intel/common/sst-dsp.h |  2 ++
 2 files changed, 25 insertions(+)

Comments

Mark Brown July 8, 2015, 6:36 p.m. UTC | #1
On Fri, Jul 03, 2015 at 04:04:02PM +0530, Vinod Koul wrote:

> +	for (time = 0; time < timeout; time++) {
> +		if ((sst_dsp_shim_read_unlocked(ctx, offset) & mask) == expected_value)
> +			break;
> +
> +		mdelay(1);
> +	}

mdelay() not msleep()?  If we're waiting for multiple miliseconds that
could be lots of busy waiting.
Vinod Koul July 9, 2015, 4:27 a.m. UTC | #2
On Wed, Jul 08, 2015 at 07:36:21PM +0100, Mark Brown wrote:
> On Fri, Jul 03, 2015 at 04:04:02PM +0530, Vinod Koul wrote:
> 
> > +	for (time = 0; time < timeout; time++) {
> > +		if ((sst_dsp_shim_read_unlocked(ctx, offset) & mask) == expected_value)
> > +			break;
> > +
> > +		mdelay(1);
> > +	}
> 
> mdelay() not msleep()?  If we're waiting for multiple miliseconds that
> could be lots of busy waiting.
Usually this should get reflected in 1st iteration as the register update
would get updated farrily quickly. msleep will add up lots of latency to
this.

Thanks
Mark Brown July 9, 2015, 10:48 a.m. UTC | #3
On Thu, Jul 09, 2015 at 09:57:41AM +0530, Vinod Koul wrote:
> On Wed, Jul 08, 2015 at 07:36:21PM +0100, Mark Brown wrote:

> > > +	for (time = 0; time < timeout; time++) {
> > > +		if ((sst_dsp_shim_read_unlocked(ctx, offset) & mask) == expected_value)
> > > +			break;
> > > +
> > > +		mdelay(1);
> > > +	}

> > mdelay() not msleep()?  If we're waiting for multiple miliseconds that
> > could be lots of busy waiting.

> Usually this should get reflected in 1st iteration as the register update
> would get updated farrily quickly. msleep will add up lots of latency to
> this.

A common approach for that is to do a busy wait for say the first
milisecond (perhaps polling more often too) and then fall back to
something sleepy if things are slow.
Vinod Koul July 9, 2015, 10:53 a.m. UTC | #4
On Thu, Jul 09, 2015 at 11:48:55AM +0100, Mark Brown wrote:
> On Thu, Jul 09, 2015 at 09:57:41AM +0530, Vinod Koul wrote:
> > On Wed, Jul 08, 2015 at 07:36:21PM +0100, Mark Brown wrote:
> 
> > > > +	for (time = 0; time < timeout; time++) {
> > > > +		if ((sst_dsp_shim_read_unlocked(ctx, offset) & mask) == expected_value)
> > > > +			break;
> > > > +
> > > > +		mdelay(1);
> > > > +	}
> 
> > > mdelay() not msleep()?  If we're waiting for multiple miliseconds that
> > > could be lots of busy waiting.
> 
> > Usually this should get reflected in 1st iteration as the register update
> > would get updated farrily quickly. msleep will add up lots of latency to
> > this.
> 
> A common approach for that is to do a busy wait for say the first
> milisecond (perhaps polling more often too) and then fall back to
> something sleepy if things are slow.
Yes that sounds sensible to me, will add

Thanks

Patch
diff mbox

diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c
index 64e94212d2d2..3356792d6933 100644
--- a/sound/soc/intel/common/sst-dsp.c
+++ b/sound/soc/intel/common/sst-dsp.c
@@ -20,6 +20,7 @@ 
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
+#include <linux/delay.h>
 
 #include "sst-dsp.h"
 #include "sst-dsp-priv.h"
@@ -222,6 +223,28 @@  int sst_dsp_shim_update_bits64(struct sst_dsp *sst, u32 offset,
 }
 EXPORT_SYMBOL_GPL(sst_dsp_shim_update_bits64);
 
+int sst_dsp_register_poll(struct sst_dsp *ctx, u32 offset, u32 mask,
+			 u32 expected_value, u32 timeout, char *operation)
+{
+	int time, ret;
+	u32 reg;
+
+	/* check if set state successful */
+	for (time = 0; time < timeout; time++) {
+		if ((sst_dsp_shim_read_unlocked(ctx, offset) & mask) == expected_value)
+			break;
+
+		mdelay(1);
+	}
+	reg = sst_dsp_shim_read_unlocked(ctx, offset);
+	dev_info(ctx->dev, "FW Poll Status: reg=%#x %s %s\n", reg, operation,
+			(time < timeout) ? "successful" : "timedout");
+	ret = time < timeout ? 0 : -ETIME;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sst_dsp_register_poll);
+
 void sst_dsp_dump(struct sst_dsp *sst)
 {
 	if (sst->ops->dump)
diff --git a/sound/soc/intel/common/sst-dsp.h b/sound/soc/intel/common/sst-dsp.h
index 96aeb2556ad4..cc3197be4cf7 100644
--- a/sound/soc/intel/common/sst-dsp.h
+++ b/sound/soc/intel/common/sst-dsp.h
@@ -278,6 +278,8 @@  void sst_dsp_inbox_read(struct sst_dsp *dsp, void *message, size_t bytes);
 void sst_dsp_outbox_write(struct sst_dsp *dsp, void *message, size_t bytes);
 void sst_dsp_outbox_read(struct sst_dsp *dsp, void *message, size_t bytes);
 void sst_dsp_mailbox_dump(struct sst_dsp *dsp, size_t bytes);
+int sst_dsp_register_poll(struct sst_dsp  *dsp, u32 offset, u32 mask,
+		 u32 expected_value, u32 timeout, char *operation);
 
 /* Debug */
 void sst_dsp_dump(struct sst_dsp *sst);