2008-06-04 15:11:10

by Guennadi Liakhovetski

[permalink] [raw]
Subject: [PATCH] spi: two bug-fixes

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 <[email protected]>

---

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;


2008-06-04 15:32:21

by Adrian Bunk

[permalink] [raw]
Subject: Re: [PATCH] spi: two bug-fixes

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 <[email protected]>
>
> ---
>
> 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

2008-06-04 15:41:19

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH] spi: two bug-fixes

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

2008-06-04 19:52:48

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH] spi: two bug-fixes

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.