[PATCH] SPI: drain MXC SPI transfer buffer when probing device

[PATCH] SPI: drain MXC SPI transfer buffer when probing device

[Daniel Mack]

On the MX31litekit, the bootloader seems to communicate with the MC13783
PMIC chip before booting Linux. However, it does not flush all the
buffers properly after that, which makes the imx-spi driver read
bogus data when probing the MC13783.

Fix that by draining the SPI buffer on startup.

Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: David Brownell <dbrownell@users.sourceforge.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: spi-devel-general@lists.sourceforge.net
---
 drivers/spi/spi_imx.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/spi_imx.c b/drivers/spi/spi_imx.c
index 89c22ef..a3894fd 100644
--- a/drivers/spi/spi_imx.c
+++ b/drivers/spi/spi_imx.c
@@ -42,8 +42,11 @@
 #define MXC_CSPITXDATA		0x04
 #define MXC_CSPICTRL		0x08
 #define MXC_CSPIINT		0x0c
+#define MXC_CSPISTAT		0x14
 #define MXC_RESET		0x1c
 
+#define MXC_CSPISTAT_RR		(1 << 3)
+
 /* generic defines to abstract from the different register layouts */
 #define MXC_INT_RR	(1 << 0) /* Receive data ready interrupt */
 #define MXC_INT_TE	(1 << 1) /* Transmit FIFO empty interrupt */
@@ -593,6 +596,10 @@ static int __init spi_imx_probe(struct platform_device *pdev)
 	if (!cpu_is_mx31() || !cpu_is_mx35())
 		writel(1, spi_imx->base + MXC_RESET);
 
+	/* drain the buffer */
+	while (readl(spi_imx->base + MXC_CSPISTAT) & MXC_CSPISTAT_RR)
+		readl(spi_imx->base + MXC_CSPIRXDATA);
+
 	spi_imx->intctrl(spi_imx, 0);
 
 	ret = spi_bitbang_start(&spi_imx->bitbang);
-- 
1.6.5.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] SPI: drain MXC SPI transfer buffer when probing device

[Uwe Kleine-König]

Hello Daniel,

On Thu, Nov 19, 2009 at 11:44:47AM +0100, Daniel Mack wrote:
> On the MX31litekit, the bootloader seems to communicate with the MC13783
> PMIC chip before booting Linux. However, it does not flush all the
> buffers properly after that, which makes the imx-spi driver read
> bogus data when probing the MC13783.
> 
> Fix that by draining the SPI buffer on startup.
> 
> Signed-off-by: Daniel Mack <daniel-rDUAYElUppE@public.gmane.org>
> Cc: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> ---
>  drivers/spi/spi_imx.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/spi/spi_imx.c b/drivers/spi/spi_imx.c
> index 89c22ef..a3894fd 100644
> --- a/drivers/spi/spi_imx.c
> +++ b/drivers/spi/spi_imx.c
> @@ -42,8 +42,11 @@
>  #define MXC_CSPITXDATA		0x04
>  #define MXC_CSPICTRL		0x08
>  #define MXC_CSPIINT		0x0c
> +#define MXC_CSPISTAT		0x14
On imx27 the register at offset 0x14 is called PERIODREG ...

>  #define MXC_RESET		0x1c
>  
> +#define MXC_CSPISTAT_RR		(1 << 3)
... and bits 0..14 are called WAIT.


> +
>  /* generic defines to abstract from the different register layouts */
>  #define MXC_INT_RR	(1 << 0) /* Receive data ready interrupt */
>  #define MXC_INT_TE	(1 << 1) /* Transmit FIFO empty interrupt */
> @@ -593,6 +596,10 @@ static int __init spi_imx_probe(struct platform_device *pdev)
>  	if (!cpu_is_mx31() || !cpu_is_mx35())
>  		writel(1, spi_imx->base + MXC_RESET);
>  
> +	/* drain the buffer */
> +	while (readl(spi_imx->base + MXC_CSPISTAT) & MXC_CSPISTAT_RR)
> +		readl(spi_imx->base + MXC_CSPIRXDATA);
> +
So this needs protection by

	if (cpu_is_mx31() || cpu_is_mx35())

(note, I didn't check the mx35 reference!).

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

Re: [PATCH] SPI: drain MXC SPI transfer buffer when probing device

[Daniel Mack]

Hi Uwe,

On Thu, Nov 19, 2009 at 07:49:51PM +0100, Uwe Kleine-König wrote:
> On Thu, Nov 19, 2009 at 11:44:47AM +0100, Daniel Mack wrote:
> > On the MX31litekit, the bootloader seems to communicate with the MC13783
> > PMIC chip before booting Linux. However, it does not flush all the
> > buffers properly after that, which makes the imx-spi driver read
> > bogus data when probing the MC13783.
> > 
> > Fix that by draining the SPI buffer on startup.
> > 
> > Signed-off-by: Daniel Mack <daniel-rDUAYElUppE@public.gmane.org>
> > Cc: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> > Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> > Cc: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > ---
> >  drivers/spi/spi_imx.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/spi/spi_imx.c b/drivers/spi/spi_imx.c
> > index 89c22ef..a3894fd 100644
> > --- a/drivers/spi/spi_imx.c
> > +++ b/drivers/spi/spi_imx.c
> > @@ -42,8 +42,11 @@
> >  #define MXC_CSPITXDATA		0x04
> >  #define MXC_CSPICTRL		0x08
> >  #define MXC_CSPIINT		0x0c
> > +#define MXC_CSPISTAT		0x14
> On imx27 the register at offset 0x14 is called PERIODREG ...
> 
> >  #define MXC_RESET		0x1c
> >  
> > +#define MXC_CSPISTAT_RR		(1 << 3)
> ... and bits 0..14 are called WAIT.
> 
> 
> > +
> >  /* generic defines to abstract from the different register layouts */
> >  #define MXC_INT_RR	(1 << 0) /* Receive data ready interrupt */
> >  #define MXC_INT_TE	(1 << 1) /* Transmit FIFO empty interrupt */
> > @@ -593,6 +596,10 @@ static int __init spi_imx_probe(struct platform_device *pdev)
> >  	if (!cpu_is_mx31() || !cpu_is_mx35())
> >  		writel(1, spi_imx->base + MXC_RESET);
> >  
> > +	/* drain the buffer */
> > +	while (readl(spi_imx->base + MXC_CSPISTAT) & MXC_CSPISTAT_RR)
> > +		readl(spi_imx->base + MXC_CSPIRXDATA);
> > +
> So this needs protection by
> 
> 	if (cpu_is_mx31() || cpu_is_mx35())

Oh, I wasn't aware of such differences, thanks for checking!

> (note, I didn't check the mx35 reference!).

But I did now, and mx35 seems to be compatible. I also changed the
register define names to avoid confusion.

New patch below.

Thanks,
Daniel


>From 965180664e33f972409fb8ae00372902406c1c22 Mon Sep 17 00:00:00 2001
From: Daniel Mack <daniel-rDUAYElUppE@public.gmane.org>
Date: Thu, 19 Nov 2009 11:39:45 +0100
Subject: [PATCH] SPI: drain MXC SPI transfer buffer when probing device
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On the MX31litekit, the bootloader seems to communicate with the MC13783
PMIC chip before booting Linux. However, it does not flush all the
buffers properly after that, which makes the imx-spi driver read
bogus data when probing the MC13783.

Fix that by draining the SPI receive buffer on startup.

Signed-off-by: Daniel Mack <daniel-rDUAYElUppE@public.gmane.org>
Cc: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Cc: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
---
 drivers/spi/spi_imx.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/spi_imx.c b/drivers/spi/spi_imx.c
index 89c22ef..1b17f44 100644
--- a/drivers/spi/spi_imx.c
+++ b/drivers/spi/spi_imx.c
@@ -44,6 +44,9 @@
 #define MXC_CSPIINT		0x0c
 #define MXC_RESET		0x1c
 
+#define MX3_CSPISTAT		0x14
+#define MX3_CSPISTAT_RR		(1 << 3)
+
 /* generic defines to abstract from the different register layouts */
 #define MXC_INT_RR	(1 << 0) /* Receive data ready interrupt */
 #define MXC_INT_TE	(1 << 1) /* Transmit FIFO empty interrupt */
@@ -593,6 +596,11 @@ static int __init spi_imx_probe(struct platform_device *pdev)
 	if (!cpu_is_mx31() || !cpu_is_mx35())
 		writel(1, spi_imx->base + MXC_RESET);
 
+	/* drain receive buffer */
+	if (cpu_is_mx31() || cpu_is_mx35())
+		while (readl(spi_imx->base + MX3_CSPISTAT) & MX3_CSPISTAT_RR)
+			readl(spi_imx->base + MXC_CSPIRXDATA);
+
 	spi_imx->intctrl(spi_imx, 0);
 
 	ret = spi_bitbang_start(&spi_imx->bitbang);
-- 
1.6.5.2


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

Re: [PATCH] SPI: drain MXC SPI transfer buffer when probing device

[Uwe Kleine-König]

Hello,

On Thu, Nov 19, 2009 at 08:01:42PM +0100, Daniel Mack wrote:
> Hi Uwe,
> 
> On Thu, Nov 19, 2009 at 07:49:51PM +0100, Uwe Kleine-König wrote:
> > On Thu, Nov 19, 2009 at 11:44:47AM +0100, Daniel Mack wrote:
> > > On the MX31litekit, the bootloader seems to communicate with the MC13783
> > > PMIC chip before booting Linux. However, it does not flush all the
> > > buffers properly after that, which makes the imx-spi driver read
> > > bogus data when probing the MC13783.
> > > 
> > > Fix that by draining the SPI buffer on startup.
> > > 
> > > Signed-off-by: Daniel Mack <daniel@caiaq.de>
> > > Cc: David Brownell <dbrownell@users.sourceforge.net>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > Cc: spi-devel-general@lists.sourceforge.net
> > > ---
> > >  drivers/spi/spi_imx.c |    7 +++++++
> > >  1 files changed, 7 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/spi/spi_imx.c b/drivers/spi/spi_imx.c
> > > index 89c22ef..a3894fd 100644
> > > --- a/drivers/spi/spi_imx.c
> > > +++ b/drivers/spi/spi_imx.c
> > > @@ -42,8 +42,11 @@
> > >  #define MXC_CSPITXDATA		0x04
> > >  #define MXC_CSPICTRL		0x08
> > >  #define MXC_CSPIINT		0x0c
> > > +#define MXC_CSPISTAT		0x14
> > On imx27 the register at offset 0x14 is called PERIODREG ...
> > 
> > >  #define MXC_RESET		0x1c
> > >  
> > > +#define MXC_CSPISTAT_RR		(1 << 3)
> > ... and bits 0..14 are called WAIT.
> > 
> > 
> > > +
> > >  /* generic defines to abstract from the different register layouts */
> > >  #define MXC_INT_RR	(1 << 0) /* Receive data ready interrupt */
> > >  #define MXC_INT_TE	(1 << 1) /* Transmit FIFO empty interrupt */
> > > @@ -593,6 +596,10 @@ static int __init spi_imx_probe(struct platform_device *pdev)
> > >  	if (!cpu_is_mx31() || !cpu_is_mx35())
> > >  		writel(1, spi_imx->base + MXC_RESET);
> > >  
> > > +	/* drain the buffer */
> > > +	while (readl(spi_imx->base + MXC_CSPISTAT) & MXC_CSPISTAT_RR)
> > > +		readl(spi_imx->base + MXC_CSPIRXDATA);
> > > +
> > So this needs protection by
> > 
> > 	if (cpu_is_mx31() || cpu_is_mx35())
> 
> Oh, I wasn't aware of such differences, thanks for checking!
> 
> > (note, I didn't check the mx35 reference!).
> 
> But I did now, and mx35 seems to be compatible. I also changed the
> register define names to avoid confusion.
hmm, I'm not really happy with the naming, but as I don't have a better
idea now, it's OK for me.
 
You can take it as an ack.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |