[PATCH] spi: two bug-fixes

[PATCH] spi: two bug-fixes

[Guennadi Liakhovetski]

Two SPI-related fixes: unsigned int cannot be less than zero, and the list
search success check is wrong: for example, it didn't recognise failure for me
when I requested port 0.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@pengutronix.de>

---

It might be a good idea to get it in for 2.6.26. As a side note I was 
surprised, that the compiler didn't generate a warning for "if (t<0)" for 
unsigned t, whereas it does generate one for unsigned char with the same 
flags (-Wall). I asked on gcc, there was one reply from Segher 
Boessenkool, it is unclear if this is going to be recognised as a bug or 
not: http://gcc.gnu.org/ml/gcc/2008-06/threads.html#00044

 arch/arm/mach-pxa/ssp.c |    2 +-
 drivers/gpio/mcp23s08.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/gpio/mcp23s08.c
===================================================================
--- linux-2.6.orig/drivers/gpio/mcp23s08.c	2008-06-04 12:11:26.000000000 +0200
+++ linux-2.6/drivers/gpio/mcp23s08.c	2008-06-04 12:11:50.000000000 +0200
@@ -178,7 +178,7 @@
 
 	mutex_lock(&mcp->lock);
 	t = mcp23s08_read_regs(mcp, 0, mcp->cache, sizeof mcp->cache);
-	if (t < 0) {
+	if ((int)t < 0) {
 		seq_printf(s, " I/O ERROR %d\n", t);
 		goto done;
 	}
Index: linux-2.6/arch/arm/mach-pxa/ssp.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-pxa/ssp.c	2008-06-04 12:11:26.000000000 +0200
+++ linux-2.6/arch/arm/mach-pxa/ssp.c	2008-06-04 12:11:50.000000000 +0200
@@ -330,7 +330,7 @@
 
 	mutex_unlock(&ssp_lock);
 
-	if (ssp->port_id != port)
+	if (&ssp->node == &ssp_list)
 		return NULL;
 
 	return ssp;

Re: [PATCH] spi: two bug-fixes

[Adrian Bunk]

On Wed, Jun 04, 2008 at 05:11:14PM +0200, Guennadi Liakhovetski wrote:
> Two SPI-related fixes: unsigned int cannot be less than zero, and the list
> search success check is wrong: for example, it didn't recognise failure for me
> when I requested port 0.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@pengutronix.de>
> 
> ---
> 
> It might be a good idea to get it in for 2.6.26. As a side note I was 
> surprised, that the compiler didn't generate a warning for "if (t<0)" for 
> unsigned t, whereas it does generate one for unsigned char with the same 
> flags (-Wall). I asked on gcc, there was one reply from Segher 
> Boessenkool, it is unclear if this is going to be recognised as a bug or 
> not: http://gcc.gnu.org/ml/gcc/2008-06/threads.html#00044
> 
>  arch/arm/mach-pxa/ssp.c |    2 +-
>  drivers/gpio/mcp23s08.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/gpio/mcp23s08.c
> ===================================================================
> --- linux-2.6.orig/drivers/gpio/mcp23s08.c	2008-06-04 12:11:26.000000000 +0200
> +++ linux-2.6/drivers/gpio/mcp23s08.c	2008-06-04 12:11:50.000000000 +0200
> @@ -178,7 +178,7 @@
>  
>  	mutex_lock(&mcp->lock);
>  	t = mcp23s08_read_regs(mcp, 0, mcp->cache, sizeof mcp->cache);
> -	if (t < 0) {
> +	if ((int)t < 0) {
>  		seq_printf(s, " I/O ERROR %d\n", t);
>  		goto done;
>  	}
>...

The better fix of making "t" signed by Roel Kluin is already as
commit 1d1c1d9b557a12320174058d2d313ffb0f8611f4 in Linus' tree.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] spi: two bug-fixes

[Guennadi Liakhovetski]

On Wed, 4 Jun 2008, Adrian Bunk wrote:

> On Wed, Jun 04, 2008 at 05:11:14PM +0200, Guennadi Liakhovetski wrote:
> > Index: linux-2.6/drivers/gpio/mcp23s08.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/gpio/mcp23s08.c	2008-06-04 12:11:26.000000000 +0200
> > +++ linux-2.6/drivers/gpio/mcp23s08.c	2008-06-04 12:11:50.000000000 +0200
> > @@ -178,7 +178,7 @@
> >  
> >  	mutex_lock(&mcp->lock);
> >  	t = mcp23s08_read_regs(mcp, 0, mcp->cache, sizeof mcp->cache);
> > -	if (t < 0) {
> > +	if ((int)t < 0) {
> >  		seq_printf(s, " I/O ERROR %d\n", t);
> >  		goto done;
> >  	}
> >...
> 
> The better fix of making "t" signed by Roel Kluin is already as
> commit 1d1c1d9b557a12320174058d2d313ffb0f8611f4 in Linus' tree.

Good, thanks for letting know! David, please, let me know if you want me 
to resubmit the second part of the patch separately, or if you can just 
extract it from the original one.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [PATCH] spi: two bug-fixes

[David Brownell]

On Wednesday 04 June 2008, Guennadi Liakhovetski wrote:
> 
> > The better fix of making "t" signed by Roel Kluin is already as
> > commit 1d1c1d9b557a12320174058d2d313ffb0f8611f4 in Linus' tree.
> 
> Good, thanks for letting know! David, please, let me know if you want me 
> to resubmit the second part of the patch separately, or if you can just 
> extract it from the original one.

Yes, please ... with updated patch comments.