FSL_SSI: fix fsl_ssi interrupt reporting so that interrupts are only reported when they have been enabled.
diff mbox

Message ID 1446061992-88379-1-git-send-email-caleb@crome.org
State Not Applicable
Headers show

Commit Message

Caleb Crome Oct. 28, 2015, 7:53 p.m. UTC
The current implementation reports all interrupt stats
in /sys/kernel/debug/*.ssi/stats, even if the interrupt
was not enabled.  This is not the intention as per
the comments.  This fix keeps track of ever bit in the
SIER register that has ever been enabled, which allows
fsl_ssi_stats_show to actually only report those
interrupt counts for interrupts that are enabled.

Signed-off-by: Caleb Crome <caleb@crome.org>
---
 sound/soc/fsl/fsl_ssi.c     | 15 +++++++++++++++
 sound/soc/fsl/fsl_ssi.h     |  1 +
 sound/soc/fsl/fsl_ssi_dbg.c |  8 +++-----
 3 files changed, 19 insertions(+), 5 deletions(-)

Comments

Fabio Estevam Oct. 28, 2015, 8:08 p.m. UTC | #1
Hi Caleb,

On Wed, Oct 28, 2015 at 5:53 PM, Caleb Crome <caleb@crome.org> wrote:

> +// call fsl_ssi_dbg_update_sier_enbled every time sier is
> +// updated.  it tracks which interrupts have ever been enabled.
> +// so that the print function can keep track.

The standard style for multi-line comments is:

/*
 * Bla bla bla
 * bla bla
 * foo foo
 */
Caleb Crome Oct. 28, 2015, 8:18 p.m. UTC | #2
On Wed, Oct 28, 2015 at 1:08 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Caleb,
>
> On Wed, Oct 28, 2015 at 5:53 PM, Caleb Crome <caleb@crome.org> wrote:
>
>> +// call fsl_ssi_dbg_update_sier_enbled every time sier is
>> +// updated.  it tracks which interrupts have ever been enabled.
>> +// so that the print function can keep track.
>
> The standard style for multi-line comments is:
>
> /*
>  * Bla bla bla
>  * bla bla
>  * foo foo
>  */
Ah, thanks.  Will fix it and resubmit.

-Caleb
Fabio Estevam Oct. 28, 2015, 8:25 p.m. UTC | #3
On Wed, Oct 28, 2015 at 6:18 PM, Caleb Crome <caleb@crome.org> wrote:
> On Wed, Oct 28, 2015 at 1:08 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> Hi Caleb,
>>
>> On Wed, Oct 28, 2015 at 5:53 PM, Caleb Crome <caleb@crome.org> wrote:
>>
>>> +// call fsl_ssi_dbg_update_sier_enbled every time sier is
>>> +// updated.  it tracks which interrupts have ever been enabled.
>>> +// so that the print function can keep track.
>>
>> The standard style for multi-line comments is:
>>
>> /*
>>  * Bla bla bla
>>  * bla bla
>>  * foo foo
>>  */
> Ah, thanks.  Will fix it and resubmit.

Some more tips:

- The Subject should be something like: ASoC: fsl_ssi: Fix fsl_ssi
interrupt reporting

- Then run  ./scripts/checkpatch.pl 0001-yourpatch.patch to see
potential issues. The // comment issue would be reported by checkpatch

- Also run  ./scripts/get_maintainer.pl 0001-yourpatch.patch so that
you get the correct people to Cc the patch too. The email address you
put for Timur is no longer valid and Nicolin must be on Cc.

Regards,

Fabio Estevam
Caleb Crome Oct. 28, 2015, 8:47 p.m. UTC | #4
On Wed, Oct 28, 2015 at 1:25 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Oct 28, 2015 at 6:18 PM, Caleb Crome <caleb@crome.org> wrote:
>> On Wed, Oct 28, 2015 at 1:08 PM, Fabio Estevam <festevam@gmail.com> wrote:
>>> Hi Caleb,
>>>
>>> On Wed, Oct 28, 2015 at 5:53 PM, Caleb Crome <caleb@crome.org> wrote:
>>>
>>>> +// call fsl_ssi_dbg_update_sier_enbled every time sier is
>>>> +// updated.  it tracks which interrupts have ever been enabled.
>>>> +// so that the print function can keep track.
>>>
>>> The standard style for multi-line comments is:
>>>
>>> /*
>>>  * Bla bla bla
>>>  * bla bla
>>>  * foo foo
>>>  */
>> Ah, thanks.  Will fix it and resubmit.
>
> Some more tips:
>
> - The Subject should be something like: ASoC: fsl_ssi: Fix fsl_ssi
> interrupt reporting
>
> - Then run  ./scripts/checkpatch.pl 0001-yourpatch.patch to see
> potential issues. The // comment issue would be reported by checkpatch
>
> - Also run  ./scripts/get_maintainer.pl 0001-yourpatch.patch so that
> you get the correct people to Cc the patch too. The email address you
> put for Timur is no longer valid and Nicolin must be on Cc.
>
> Regards,
>
> Fabio Estevam

Great.  Thank you, that will make things go more smoothly for me.

Thanks,
 -Caleb
Timur Tabi Oct. 28, 2015, 10:39 p.m. UTC | #5
On Wed, Oct 28, 2015 at 2:53 PM, Caleb Crome <caleb@crome.org> wrote:
> The current implementation reports all interrupt stats
> in /sys/kernel/debug/*.ssi/stats, even if the interrupt
> was not enabled.  This is not the intention as per
> the comments.  This fix keeps track of ever bit in the

"every"

Also, please send patches to timur@tabi.org and not
timur@freescale.com.  Thanks.

Patch
diff mbox

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 73778c2..14d5ade 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -299,6 +299,18 @@  static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+// call fsl_ssi_dbg_update_sier_enbled every time sier is
+// updated.  it tracks which interrupts have ever been enabled.
+// so that the print function can keep track.
+void fsl_ssi_update_sier_enabled(
+	struct fsl_ssi_private *ssi_private)
+{
+	int sier;
+	struct regmap *regs = ssi_private->regs;
+	regmap_read(regs, CCSR_SSI_SIER, &sier);
+	ssi_private->dbg_stats.sier_ever_enabled |= sier;
+}
+
 /*
  * Enable/Disable all rx/tx config flags at once.
  */
@@ -312,6 +324,7 @@  static void fsl_ssi_rxtx_config(struct fsl_ssi_private *ssi_private,
 		regmap_update_bits(regs, CCSR_SSI_SIER,
 				vals->rx.sier | vals->tx.sier,
 				vals->rx.sier | vals->tx.sier);
+		fsl_ssi_update_sier_enabled(ssi_private);
 		regmap_update_bits(regs, CCSR_SSI_SRCR,
 				vals->rx.srcr | vals->tx.srcr,
 				vals->rx.srcr | vals->tx.srcr);
@@ -404,6 +417,7 @@  static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable,
 	 */
 	if (enable) {
 		regmap_update_bits(regs, CCSR_SSI_SIER, vals->sier, vals->sier);
+		fsl_ssi_update_sier_enabled(ssi_private);
 		regmap_update_bits(regs, CCSR_SSI_SRCR, vals->srcr, vals->srcr);
 		regmap_update_bits(regs, CCSR_SSI_STCR, vals->stcr, vals->stcr);
 	} else {
@@ -431,6 +445,7 @@  static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable,
 		regmap_update_bits(regs, CCSR_SSI_SRCR, srcr, 0);
 		regmap_update_bits(regs, CCSR_SSI_STCR, stcr, 0);
 		regmap_update_bits(regs, CCSR_SSI_SIER, sier, 0);
+		fsl_ssi_update_sier_enabled(ssi_private);
 	}
 
 config_done:
diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h
index 5065105..a078c69 100644
--- a/sound/soc/fsl/fsl_ssi.h
+++ b/sound/soc/fsl/fsl_ssi.h
@@ -237,6 +237,7 @@  struct fsl_ssi_dbg {
 		unsigned int tfe1;
 		unsigned int tfe0;
 	} stats;
+	unsigned int sier_ever_enabled;
 };
 
 void fsl_ssi_dbg_isr(struct fsl_ssi_dbg *ssi_dbg, u32 sisr);
diff --git a/sound/soc/fsl/fsl_ssi_dbg.c b/sound/soc/fsl/fsl_ssi_dbg.c
index 5469ffb..1066620 100644
--- a/sound/soc/fsl/fsl_ssi_dbg.c
+++ b/sound/soc/fsl/fsl_ssi_dbg.c
@@ -82,13 +82,10 @@  void fsl_ssi_dbg_isr(struct fsl_ssi_dbg *dbg, u32 sisr)
 		dbg->stats.tfe0++;
 }
 
-/* Show the statistics of a flag only if its interrupt is enabled.  The
- * compiler will optimze this code to a no-op if the interrupt is not
- * enabled.
- */
+/* Show the statistics of a flag only if its interrupt is enabled. */
 #define SIER_SHOW(flag, name) \
 	do { \
-		if (CCSR_SSI_SIER_##flag) \
+		if (CCSR_SSI_SIER_##flag & sier) \
 			seq_printf(s, #name "=%u\n", ssi_dbg->stats.name); \
 	} while (0)
 
@@ -102,6 +99,7 @@  void fsl_ssi_dbg_isr(struct fsl_ssi_dbg *dbg, u32 sisr)
 static int fsl_ssi_stats_show(struct seq_file *s, void *unused)
 {
 	struct fsl_ssi_dbg *ssi_dbg = s->private;
+	unsigned int sier = ssi_dbg->sier_ever_enabled;
 
 	SIER_SHOW(RFRC_EN, rfrc);
 	SIER_SHOW(TFRC_EN, tfrc);