diff mbox

ASoC: fsl_sai: Add isr to deal with error flag

Message ID 1395834517-16426-1-git-send-email-Guangyu.Chen@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nicolin Chen March 26, 2014, 11:48 a.m. UTC
It's quite cricial to clear error flags because SAI might hang if getting
FIFO underrun during playback (I haven't confirmed the same issue on Rx
overflow though).

So this patch enables those irq and adds isr() to clear the flags so as to
keep playback entirely safe.

Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
---
 sound/soc/fsl/fsl_sai.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++---
 sound/soc/fsl/fsl_sai.h |  9 +++++++
 2 files changed, 75 insertions(+), 3 deletions(-)

Comments

David Laight March 26, 2014, 11:59 a.m. UTC | #1
From: Nicolin Chen
> It's quite cricial to clear error flags because SAI might hang if getting
> FIFO underrun during playback (I haven't confirmed the same issue on Rx
> overflow though).
> 
> So this patch enables those irq and adds isr() to clear the flags so as to
> keep playback entirely safe.
> 
> Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> ---
>  sound/soc/fsl/fsl_sai.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++---
>  sound/soc/fsl/fsl_sai.h |  9 +++++++
>  2 files changed, 75 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index c4a4231..5f91aff 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -23,6 +23,55 @@
> 
>  #include "fsl_sai.h"
> 
> +#define FSL_SAI_FLAGS (FSL_SAI_CSR_SEIE |\
> +		       FSL_SAI_CSR_FEIE |\
> +		       FSL_SAI_CSR_FWIE)
> +
> +static irqreturn_t fsl_sai_isr(int irq, void *devid)
> +{
> +	struct fsl_sai *sai = (struct fsl_sai *)devid;
> +	struct device *dev = &sai->pdev->dev;
> +	u32 xcsr;
> +
> +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);

Assuming these are 'write to clear' bits, you might want
to make the write (above) and all the traces (below)
conditional on the value being non-zero.

> +	if (xcsr & FSL_SAI_CSR_WSF)
> +		dev_dbg(dev, "isr: Start of Tx word detected\n");
> +
> +	if (xcsr & FSL_SAI_CSR_SEF)
> +		dev_dbg(dev, "isr: Tx Frame sync error detected\n");
> +
> +	if (xcsr & FSL_SAI_CSR_FEF)
> +		dev_dbg(dev, "isr: Transmit underrun detected\n");
> +
> +	if (xcsr & FSL_SAI_CSR_FWF)
> +		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
> +
> +	if (xcsr & FSL_SAI_CSR_FRF)
> +		dev_dbg(dev, "isr: Transmit FIFO watermark has been reached\n");

Some of those look like 'normal' interrupts, others are clearly
abnormal conditions.
Maybe the tracing should reflect this.

> +
> +	regmap_read(sai->regmap, FSL_SAI_RCSR, &xcsr);
> +	regmap_write(sai->regmap, FSL_SAI_RCSR, xcsr);

Same comments apply...

	David
Mark Brown March 27, 2014, 1:14 a.m. UTC | #2
On Wed, Mar 26, 2014 at 11:59:53AM +0000, David Laight wrote:
> From: Nicolin Chen

> > +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> > +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);

> Assuming these are 'write to clear' bits, you might want
> to make the write (above) and all the traces (below)
> conditional on the value being non-zero.

The trace is already conditional?  I'd also expect to see the driver
only acknowledging sources it knows about and only reporting that the
interrupt was handled if it saw one of them - right now all interrupts
are unconditionally acknowleged.
Xiubo Li March 27, 2014, 2:13 a.m. UTC | #3
> +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);
> +
> +	if (xcsr & FSL_SAI_CSR_WSF)
> +		dev_dbg(dev, "isr: Start of Tx word detected\n");
> +
> +	if (xcsr & FSL_SAI_CSR_SEF)
> +		dev_dbg(dev, "isr: Tx Frame sync error detected\n");
> +
> +	if (xcsr & FSL_SAI_CSR_FEF)
> +		dev_dbg(dev, "isr: Transmit underrun detected\n");
> +

Actually, the above three isrs should to write a logic 1 to this field
to clear this flag.


> +	if (xcsr & FSL_SAI_CSR_FWF)
> +		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
> +
> +	if (xcsr & FSL_SAI_CSR_FRF)
> +		dev_dbg(dev, "isr: Transmit FIFO watermark has been reached\n");
> +

While are these ones really needed to clear manually ?


Thanks,
--

Best Regards,
Xiubo
Nicolin Chen March 27, 2014, 2:29 a.m. UTC | #4
On Thu, Mar 27, 2014 at 01:14:24AM +0000, Mark Brown wrote:
> On Wed, Mar 26, 2014 at 11:59:53AM +0000, David Laight wrote:
> > From: Nicolin Chen
> 
> > > +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> > > +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);
> 
> > Assuming these are 'write to clear' bits, you might want
> > to make the write (above) and all the traces (below)
> > conditional on the value being non-zero.
> 
> The trace is already conditional?  I'd also expect to see the driver
> only acknowledging sources it knows about and only reporting that the
> interrupt was handled if it saw one of them - right now all interrupts
> are unconditionally acknowleged.

Will revise it based on the comments from both of you.

Thank you.


> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Nicolin Chen March 27, 2014, 2:36 a.m. UTC | #5
On Thu, Mar 27, 2014 at 10:13:48AM +0800, Xiubo Li-B47053 wrote:
> > +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> > +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);
> > +
> > +	if (xcsr & FSL_SAI_CSR_WSF)
> > +		dev_dbg(dev, "isr: Start of Tx word detected\n");
> > +
> > +	if (xcsr & FSL_SAI_CSR_SEF)
> > +		dev_dbg(dev, "isr: Tx Frame sync error detected\n");
> > +
> > +	if (xcsr & FSL_SAI_CSR_FEF)
> > +		dev_dbg(dev, "isr: Transmit underrun detected\n");
> > +
> 
> Actually, the above three isrs should to write a logic 1 to this field
> to clear this flag.
> 
> 
> > +	if (xcsr & FSL_SAI_CSR_FWF)
> > +		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
> > +
> > +	if (xcsr & FSL_SAI_CSR_FRF)
> > +		dev_dbg(dev, "isr: Transmit FIFO watermark has been reached\n");
> > +
> 
> While are these ones really needed to clear manually ?

The reference manual doesn't mention about the requirement. So SAI should do
the self-clearance.
Xiubo Li March 27, 2014, 2:53 a.m. UTC | #6
> On Thu, Mar 27, 2014 at 10:13:48AM +0800, Xiubo Li-B47053 wrote:
> > > +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> > > +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);
> > > +
> > > +	if (xcsr & FSL_SAI_CSR_WSF)
> > > +		dev_dbg(dev, "isr: Start of Tx word detected\n");
> > > +
> > > +	if (xcsr & FSL_SAI_CSR_SEF)
> > > +		dev_dbg(dev, "isr: Tx Frame sync error detected\n");
> > > +
> > > +	if (xcsr & FSL_SAI_CSR_FEF)
> > > +		dev_dbg(dev, "isr: Transmit underrun detected\n");
> > > +
> >
> > Actually, the above three isrs should to write a logic 1 to this field
> > to clear this flag.
> >
> >
> > > +	if (xcsr & FSL_SAI_CSR_FWF)
> > > +		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
> > > +
> > > +	if (xcsr & FSL_SAI_CSR_FRF)
> > > +		dev_dbg(dev, "isr: Transmit FIFO watermark has been reached\n");
> > > +
> >
> > While are these ones really needed to clear manually ?
> 
> The reference manual doesn't mention about the requirement. So SAI should do
> the self-clearance.

Yes, I do think we should let it do the self-clearance, and shouldn't interfere
of them...
Nicolin Chen March 27, 2014, 2:56 a.m. UTC | #7
On Thu, Mar 27, 2014 at 10:53:50AM +0800, Xiubo Li-B47053 wrote:
> > On Thu, Mar 27, 2014 at 10:13:48AM +0800, Xiubo Li-B47053 wrote:
> > > > +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> > > > +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);
> > > > +
> > > > +	if (xcsr & FSL_SAI_CSR_WSF)
> > > > +		dev_dbg(dev, "isr: Start of Tx word detected\n");
> > > > +
> > > > +	if (xcsr & FSL_SAI_CSR_SEF)
> > > > +		dev_dbg(dev, "isr: Tx Frame sync error detected\n");
> > > > +
> > > > +	if (xcsr & FSL_SAI_CSR_FEF)
> > > > +		dev_dbg(dev, "isr: Transmit underrun detected\n");
> > > > +
> > >
> > > Actually, the above three isrs should to write a logic 1 to this field
> > > to clear this flag.
> > >
> > >
> > > > +	if (xcsr & FSL_SAI_CSR_FWF)
> > > > +		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
> > > > +
> > > > +	if (xcsr & FSL_SAI_CSR_FRF)
> > > > +		dev_dbg(dev, "isr: Transmit FIFO watermark has been reached\n");
> > > > +
> > >
> > > While are these ones really needed to clear manually ?
> > 
> > The reference manual doesn't mention about the requirement. So SAI should do
> > the self-clearance.
> 
> Yes, I do think we should let it do the self-clearance, and shouldn't interfere
> of them...

SAI is supposed to ignore the interference, isn't it?
Nicolin Chen March 27, 2014, 3:31 a.m. UTC | #8
On Thu, Mar 27, 2014 at 11:41:02AM +0800, Xiubo Li-B47053 wrote:
> 
> > Subject: Re: [PATCH] ASoC: fsl_sai: Add isr to deal with error flag
> > 
> > On Thu, Mar 27, 2014 at 10:53:50AM +0800, Xiubo Li-B47053 wrote:
> > > > On Thu, Mar 27, 2014 at 10:13:48AM +0800, Xiubo Li-B47053 wrote:
> > > > > > +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> > > > > > +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);
> > > > > > +
> > > > > > +	if (xcsr & FSL_SAI_CSR_WSF)
> > > > > > +		dev_dbg(dev, "isr: Start of Tx word detected\n");
> > > > > > +
> > > > > > +	if (xcsr & FSL_SAI_CSR_SEF)
> > > > > > +		dev_dbg(dev, "isr: Tx Frame sync error detected\n");
> > > > > > +
> > > > > > +	if (xcsr & FSL_SAI_CSR_FEF)
> > > > > > +		dev_dbg(dev, "isr: Transmit underrun detected\n");
> > > > > > +
> > > > >
> > > > > Actually, the above three isrs should to write a logic 1 to this field
> > > > > to clear this flag.
> > > > >
> > > > >
> > > > > > +	if (xcsr & FSL_SAI_CSR_FWF)
> > > > > > +		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
> > > > > > +
> > > > > > +	if (xcsr & FSL_SAI_CSR_FRF)
> > > > > > +		dev_dbg(dev, "isr: Transmit FIFO watermark has been
> > reached\n");
> > > > > > +
> > > > >
> > > > > While are these ones really needed to clear manually ?
> > > >
> > > > The reference manual doesn't mention about the requirement. So SAI should
> > do
> > > > the self-clearance.
> > >
> > > Yes, I do think we should let it do the self-clearance, and shouldn't
> > interfere
> > > of them...
> > 
> > SAI is supposed to ignore the interference, isn't it?
> > 
> 
> Maybe, but I'm not very sure.
> And these bits are all writable and readable.

Double-confirmed? Because FWF and FRF should be read-only bits.
Xiubo Li March 27, 2014, 3:41 a.m. UTC | #9
> Subject: Re: [PATCH] ASoC: fsl_sai: Add isr to deal with error flag
> 
> On Thu, Mar 27, 2014 at 10:53:50AM +0800, Xiubo Li-B47053 wrote:
> > > On Thu, Mar 27, 2014 at 10:13:48AM +0800, Xiubo Li-B47053 wrote:
> > > > > +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> > > > > +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);
> > > > > +
> > > > > +	if (xcsr & FSL_SAI_CSR_WSF)
> > > > > +		dev_dbg(dev, "isr: Start of Tx word detected\n");
> > > > > +
> > > > > +	if (xcsr & FSL_SAI_CSR_SEF)
> > > > > +		dev_dbg(dev, "isr: Tx Frame sync error detected\n");
> > > > > +
> > > > > +	if (xcsr & FSL_SAI_CSR_FEF)
> > > > > +		dev_dbg(dev, "isr: Transmit underrun detected\n");
> > > > > +
> > > >
> > > > Actually, the above three isrs should to write a logic 1 to this field
> > > > to clear this flag.
> > > >
> > > >
> > > > > +	if (xcsr & FSL_SAI_CSR_FWF)
> > > > > +		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
> > > > > +
> > > > > +	if (xcsr & FSL_SAI_CSR_FRF)
> > > > > +		dev_dbg(dev, "isr: Transmit FIFO watermark has been
> reached\n");
> > > > > +
> > > >
> > > > While are these ones really needed to clear manually ?
> > >
> > > The reference manual doesn't mention about the requirement. So SAI should
> do
> > > the self-clearance.
> >
> > Yes, I do think we should let it do the self-clearance, and shouldn't
> interfere
> > of them...
> 
> SAI is supposed to ignore the interference, isn't it?
> 

Maybe, but I'm not very sure.
And these bits are all writable and readable.
Nicolin Chen March 27, 2014, 3:57 a.m. UTC | #10
On Thu, Mar 27, 2014 at 12:06:53PM +0800, Xiubo Li-B47053 wrote:
> > > > > > > > +	if (xcsr & FSL_SAI_CSR_FWF)
> > > > > > > > +		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
> > > > > > > > +
> > > > > > > > +	if (xcsr & FSL_SAI_CSR_FRF)
> > > > > > > > +		dev_dbg(dev, "isr: Transmit FIFO watermark has been
> > > > reached\n");
> > > > > > > > +
> > > > > > >
> > > > > > > While are these ones really needed to clear manually ?
> > > > > >
> > > > > > The reference manual doesn't mention about the requirement. So SAI
> > should
> > > > do
> > > > > > the self-clearance.
> > > > >
> > > > > Yes, I do think we should let it do the self-clearance, and shouldn't
> > > > interfere
> > > > > of them...
> > > >
> > > > SAI is supposed to ignore the interference, isn't it?
> > > >
> > >
> > > Maybe, but I'm not very sure.
> > > And these bits are all writable and readable.
> > 
> > Double-confirmed? Because FWF and FRF should be read-only bits.
> > 
> 
> So let's just ignore the clearance of these bits in isr().
> 
> +++++
> SAI Transmit Control Register (I2S1_TCSR) : 32 : R/W : 0000_0000h

I'm talking about FWF and FRF bits, not TCSR as a register.

> -----
> 
> I have checked in the Vybrid and LS1 SoC datasheets, and they are all the
> Same as above, and nothing else.
> 
> Have I missed ?

What i.MX IC team told me is SAI ignores what we do to FWF and FRF, so you
don't need to worry about it at all unless Vybrid makes them writable, in
which case we may also need to clear these bits and confirm with Vybrid IC
team if they're also W1C.
Xiubo Li March 27, 2014, 4:06 a.m. UTC | #11
> > > > > > > +	if (xcsr & FSL_SAI_CSR_FWF)
> > > > > > > +		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
> > > > > > > +
> > > > > > > +	if (xcsr & FSL_SAI_CSR_FRF)
> > > > > > > +		dev_dbg(dev, "isr: Transmit FIFO watermark has been
> > > reached\n");
> > > > > > > +
> > > > > >
> > > > > > While are these ones really needed to clear manually ?
> > > > >
> > > > > The reference manual doesn't mention about the requirement. So SAI
> should
> > > do
> > > > > the self-clearance.
> > > >
> > > > Yes, I do think we should let it do the self-clearance, and shouldn't
> > > interfere
> > > > of them...
> > >
> > > SAI is supposed to ignore the interference, isn't it?
> > >
> >
> > Maybe, but I'm not very sure.
> > And these bits are all writable and readable.
> 
> Double-confirmed? Because FWF and FRF should be read-only bits.
> 

So let's just ignore the clearance of these bits in isr().

+++++
SAI Transmit Control Register (I2S1_TCSR) : 32 : R/W : 0000_0000h
-----

I have checked in the Vybrid and LS1 SoC datasheets, and they are all the
Same as above, and nothing else.

Have I missed ?
Xiubo Li March 27, 2014, 4:18 a.m. UTC | #12
> > So let's just ignore the clearance of these bits in isr().
> >
> > +++++
> > SAI Transmit Control Register (I2S1_TCSR) : 32 : R/W : 0000_0000h
> 
> I'm talking about FWF and FRF bits, not TCSR as a register.
> 
> > -----
> >
> > I have checked in the Vybrid and LS1 SoC datasheets, and they are all the
> > Same as above, and nothing else.
> >
> > Have I missed ?
> 
> What i.MX IC team told me is SAI ignores what we do to FWF and FRF, so you
> don't need to worry about it at all unless Vybrid makes them writable, in
> which case we may also need to clear these bits and confirm with Vybrid IC
> team if they're also W1C.
> 

Well, if so, that's fine.

Thanks,
David Laight March 27, 2014, 9:41 a.m. UTC | #13
From: Mark Brown
> On Wed, Mar 26, 2014 at 11:59:53AM +0000, David Laight wrote:
> > From: Nicolin Chen
> 
> > > +	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
> > > +	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);
> 
> > Assuming these are 'write to clear' bits, you might want
> > to make the write (above) and all the traces (below)
> > conditional on the value being non-zero.
> 
> The trace is already conditional?  I'd also expect to see the driver
> only acknowledging sources it knows about and only reporting that the
> interrupt was handled if it saw one of them - right now all interrupts
> are unconditionally acknowleged.

The traces are separately conditional on their own bits.
That is a lot of checks that will normally be false.

Also the driver may need to clear all the active interrupt
bits in order to make the IRQ go away.
It should trace that bits it doesn't expect to be set though.

	David
Mark Brown March 27, 2014, 10:54 a.m. UTC | #14
On Thu, Mar 27, 2014 at 11:57:27AM +0800, Nicolin Chen wrote:
> On Thu, Mar 27, 2014 at 12:06:53PM +0800, Xiubo Li-B47053 wrote:

> > I have checked in the Vybrid and LS1 SoC datasheets, and they are all the
> > Same as above, and nothing else.

> > Have I missed ?

> What i.MX IC team told me is SAI ignores what we do to FWF and FRF, so you
> don't need to worry about it at all unless Vybrid makes them writable, in
> which case we may also need to clear these bits and confirm with Vybrid IC
> team if they're also W1C.

And even if it payed attention I'd expect that a lot of the time they'd
just be reasserted immediately as the condition still holds.
Mark Brown March 27, 2014, 1:05 p.m. UTC | #15
On Thu, Mar 27, 2014 at 09:41:20AM +0000, David Laight wrote:
> From: Mark Brown

> > The trace is already conditional?  I'd also expect to see the driver
> > only acknowledging sources it knows about and only reporting that the
> > interrupt was handled if it saw one of them - right now all interrupts
> > are unconditionally acknowleged.

> The traces are separately conditional on their own bits.
> That is a lot of checks that will normally be false.

Oh, I see what you mean.  I'm not sure that level of optimisation is
worth it, the overhead of taking an interrupt in the first place is
going to dwarf the optimisation and the indentation probably won't help
writing clear messages.

> Also the driver may need to clear all the active interrupt
> bits in order to make the IRQ go away.
> It should trace that bits it doesn't expect to be set though.

The interrupt core will eventually take care of it if the interrupt is
left screaming with nothing handling it (and provide diagnostics).  I
would *hope* that this is only happening for unmasked interrupt sources
we explicitly unmask anyway, we really don't want interrupts on FIFO
level changes.
diff mbox

Patch

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index c4a4231..5f91aff 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -23,6 +23,55 @@ 
 
 #include "fsl_sai.h"
 
+#define FSL_SAI_FLAGS (FSL_SAI_CSR_SEIE |\
+		       FSL_SAI_CSR_FEIE |\
+		       FSL_SAI_CSR_FWIE)
+
+static irqreturn_t fsl_sai_isr(int irq, void *devid)
+{
+	struct fsl_sai *sai = (struct fsl_sai *)devid;
+	struct device *dev = &sai->pdev->dev;
+	u32 xcsr;
+
+	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
+	regmap_write(sai->regmap, FSL_SAI_TCSR, xcsr);
+
+	if (xcsr & FSL_SAI_CSR_WSF)
+		dev_dbg(dev, "isr: Start of Tx word detected\n");
+
+	if (xcsr & FSL_SAI_CSR_SEF)
+		dev_dbg(dev, "isr: Tx Frame sync error detected\n");
+
+	if (xcsr & FSL_SAI_CSR_FEF)
+		dev_dbg(dev, "isr: Transmit underrun detected\n");
+
+	if (xcsr & FSL_SAI_CSR_FWF)
+		dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n");
+
+	if (xcsr & FSL_SAI_CSR_FRF)
+		dev_dbg(dev, "isr: Transmit FIFO watermark has been reached\n");
+
+	regmap_read(sai->regmap, FSL_SAI_RCSR, &xcsr);
+	regmap_write(sai->regmap, FSL_SAI_RCSR, xcsr);
+
+	if (xcsr & FSL_SAI_CSR_WSF)
+		dev_dbg(dev, "isr: Start of Rx word detected\n");
+
+	if (xcsr & FSL_SAI_CSR_SEF)
+		dev_dbg(dev, "isr: Rx Frame sync error detected\n");
+
+	if (xcsr & FSL_SAI_CSR_FEF)
+		dev_dbg(dev, "isr: Receive overflow detected\n");
+
+	if (xcsr & FSL_SAI_CSR_FWF)
+		dev_dbg(dev, "isr: Enabled receive FIFO is full\n");
+
+	if (xcsr & FSL_SAI_CSR_FRF)
+		dev_dbg(dev, "isr: Receive FIFO watermark has been reached\n");
+
+	return IRQ_HANDLED;
+}
+
 static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai,
 		int clk_id, unsigned int freq, int fsl_dir)
 {
@@ -373,8 +422,8 @@  static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
 {
 	struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
 
-	regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, 0x0);
-	regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, 0x0);
+	regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, FSL_SAI_FLAGS);
+	regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, FSL_SAI_FLAGS);
 	regmap_update_bits(sai->regmap, FSL_SAI_TCR1, FSL_SAI_CR1_RFW_MASK,
 			   FSL_SAI_MAXBURST_TX * 2);
 	regmap_update_bits(sai->regmap, FSL_SAI_RCR1, FSL_SAI_CR1_RFW_MASK,
@@ -490,12 +539,14 @@  static int fsl_sai_probe(struct platform_device *pdev)
 	struct fsl_sai *sai;
 	struct resource *res;
 	void __iomem *base;
-	int ret;
+	int irq, ret;
 
 	sai = devm_kzalloc(&pdev->dev, sizeof(*sai), GFP_KERNEL);
 	if (!sai)
 		return -ENOMEM;
 
+	sai->pdev = pdev;
+
 	sai->big_endian_regs = of_property_read_bool(np, "big-endian-regs");
 	if (sai->big_endian_regs)
 		fsl_sai_regmap_config.val_format_endian = REGMAP_ENDIAN_BIG;
@@ -514,6 +565,18 @@  static int fsl_sai_probe(struct platform_device *pdev)
 		return PTR_ERR(sai->regmap);
 	}
 
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "no irq for node %s\n", np->full_name);
+		return irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, fsl_sai_isr, 0, np->name, sai);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to claim irq %u\n", irq);
+		return ret;
+	}
+
 	sai->dma_params_rx.addr = res->start + FSL_SAI_RDR;
 	sai->dma_params_tx.addr = res->start + FSL_SAI_TDR;
 	sai->dma_params_rx.maxburst = FSL_SAI_MAXBURST_RX;
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index e432260..05d1a1f 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -37,7 +37,15 @@ 
 
 /* SAI Transmit/Recieve Control Register */
 #define FSL_SAI_CSR_TERE	BIT(31)
+#define FSL_SAI_CSR_WSF		BIT(20)
+#define FSL_SAI_CSR_SEF		BIT(19)
+#define FSL_SAI_CSR_FEF		BIT(18)
 #define FSL_SAI_CSR_FWF		BIT(17)
+#define FSL_SAI_CSR_FRF		BIT(16)
+#define FSL_SAI_CSR_WSIE	BIT(12)
+#define FSL_SAI_CSR_SEIE	BIT(11)
+#define FSL_SAI_CSR_FEIE	BIT(10)
+#define FSL_SAI_CSR_FWIE	BIT(9)
 #define FSL_SAI_CSR_FRIE	BIT(8)
 #define FSL_SAI_CSR_FRDE	BIT(0)
 
@@ -99,6 +107,7 @@ 
 #define FSL_SAI_MAXBURST_RX 6
 
 struct fsl_sai {
+	struct platform_device *pdev;
 	struct regmap *regmap;
 
 	bool big_endian_regs;