2006-08-06 20:22:21

by Pierre Ossman

[permalink] [raw]
Subject: [PATCH] [MMC] Fix base address configuration in wbsd

There were some confusion about base I/O variables in the wbsd driver.
Seems like things have been working on shear luck so far. The global 'io'
variable (used when manually configuring the resources) was used instead of
the local 'base' variable.

Signed-off-by: Pierre Ossman <[email protected]>
---

drivers/mmc/wbsd.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/wbsd.c b/drivers/mmc/wbsd.c
index 8a30ef3..ce86887 100644
--- a/drivers/mmc/wbsd.c
+++ b/drivers/mmc/wbsd.c
@@ -41,7 +41,7 @@ #include <asm/scatterlist.h>
#include "wbsd.h"

#define DRIVER_NAME "wbsd"
-#define DRIVER_VERSION "1.5"
+#define DRIVER_VERSION "1.6"

#define DBG(x...) \
pr_debug(DRIVER_NAME ": " x)
@@ -1439,13 +1439,13 @@ static int __devinit wbsd_scan(struct wb

static int __devinit wbsd_request_region(struct wbsd_host *host, int base)
{
- if (io & 0x7)
+ if (base & 0x7)
return -EINVAL;

if (!request_region(base, 8, DRIVER_NAME))
return -EIO;

- host->base = io;
+ host->base = base;

return 0;
}


2006-08-06 20:48:51

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

On Sun, Aug 06, 2006 at 10:22:23PM +0200, Pierre Ossman wrote:
> There were some confusion about base I/O variables in the wbsd driver.
> Seems like things have been working on shear luck so far. The global 'io'
> variable (used when manually configuring the resources) was used instead of
> the local 'base' variable.

Applied, thanks.

Shouldn't "base" be something other than "int" (eg, unsigned long) ?
Also, wbsd_init() takes base, irq, dma but passes wbsd_request_resources
io, irq and dma? I suspect more fixes are on their way... 8)

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2006-08-06 20:57:31

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

Russell King wrote:
> On Sun, Aug 06, 2006 at 10:22:23PM +0200, Pierre Ossman wrote:
>> There were some confusion about base I/O variables in the wbsd driver.
>> Seems like things have been working on shear luck so far. The global 'io'
>> variable (used when manually configuring the resources) was used instead of
>> the local 'base' variable.
>
> Applied, thanks.
>
> Shouldn't "base" be something other than "int" (eg, unsigned long) ?

unsigned short would probably be the right<tm> thing as the resource is
16 bits. I haven't seen it as big enough issue to warrant a patch.

> Also, wbsd_init() takes base, irq, dma but passes wbsd_request_resources
> io, irq and dma? I suspect more fixes are on their way... 8)

*sigh*

And I thought this brown paper bag was just temporary...

Rgds
Pierre

2006-08-06 21:05:19

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

On Sun, Aug 06, 2006 at 10:57:35PM +0200, Pierre Ossman wrote:
> Russell King wrote:
> > On Sun, Aug 06, 2006 at 10:22:23PM +0200, Pierre Ossman wrote:
> >> There were some confusion about base I/O variables in the wbsd driver.
> >> Seems like things have been working on shear luck so far. The global 'io'
> >> variable (used when manually configuring the resources) was used instead of
> >> the local 'base' variable.
> >
> > Applied, thanks.
> >
> > Shouldn't "base" be something other than "int" (eg, unsigned long) ?
>
> unsigned short would probably be the right<tm> thing as the resource is
> 16 bits. I haven't seen it as big enough issue to warrant a patch.

Is that a safe assumption to make? Is this only ever going to appear/be
used on x86?

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2006-08-06 21:10:28

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

On Sun, 06 Aug 2006 22:57:35 +0200 Pierre Ossman wrote:

> Russell King wrote:
> > On Sun, Aug 06, 2006 at 10:22:23PM +0200, Pierre Ossman wrote:
> >> There were some confusion about base I/O variables in the wbsd driver.
> >> Seems like things have been working on shear luck so far. The global 'io'
> >> variable (used when manually configuring the resources) was used instead of
> >> the local 'base' variable.
> >
> > Applied, thanks.
> >
> > Shouldn't "base" be something other than "int" (eg, unsigned long) ?
>
> unsigned short would probably be the right<tm> thing as the resource is
> 16 bits. I haven't seen it as big enough issue to warrant a patch.

and why not <resource_size_t> ?

> > Also, wbsd_init() takes base, irq, dma but passes wbsd_request_resources
> > io, irq and dma? I suspect more fixes are on their way... 8)
>
> *sigh*
>
> And I thought this brown paper bag was just temporary...


---
~Randy

2006-08-06 21:29:52

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

Russell King wrote:
> Is that a safe assumption to make? Is this only ever going to appear/be
> used on x86?
>
>

It's designed for ISA, so I think so. In the event that a version of
this model appears that has a larger I/O base, then the configure
registers will have been reordered (to make room for any extra address
bytes), so the driver will not work out-of-box anyway.

Rgds
Pierre

2006-08-06 21:30:55

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

Randy.Dunlap wrote:
> and why not <resource_size_t> ?
>
>

It would still need some casting someplace since we need to stuff the
address into two 8-bit fields when configuring the chip.

Rgds
Pierre

2006-08-06 21:35:35

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

On Sun, Aug 06, 2006 at 11:29:49PM +0200, Pierre Ossman wrote:
> Russell King wrote:
> > Is that a safe assumption to make? Is this only ever going to appear/be
> > used on x86?
> >
> >
>
> It's designed for ISA, so I think so. In the event that a version of
> this model appears that has a larger I/O base, then the configure
> registers will have been reordered (to make room for any extra address
> bytes), so the driver will not work out-of-box anyway.

ISA is very easy to glue to the simple IO busses on many
systems such as ARM.

--
Ben ([email protected], http://www.fluff.org/)

'a smiley only costs 4 bytes'

2006-08-06 21:52:17

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

Ben Dooks wrote:
> ISA is very easy to glue to the simple IO busses on many
> systems such as ARM.
>
>

Sorry, my intention wasn't to assert that it was only to be used on x86
but that the 16-bit assumption was safe.

2006-08-06 23:13:17

by Alan

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

Ar Sul, 2006-08-06 am 23:52 +0200, ysgrifennodd Pierre Ossman:
> Sorry, my intention wasn't to assert that it was only to be used on x86
> but that the 16-bit assumption was safe.

Your ISA bus mappings on a non x86 processor are likely to be 32bit MMIO
-> PIO windows in memory space.

Alan

2006-08-06 23:19:55

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Fix base address configuration in wbsd

Alan Cox wrote:
> Ar Sul, 2006-08-06 am 23:52 +0200, ysgrifennodd Pierre Ossman:
>
>> Sorry, my intention wasn't to assert that it was only to be used on x86
>> but that the 16-bit assumption was safe.
>>
>
> Your ISA bus mappings on a non x86 processor are likely to be 32bit MMIO
> -> PIO windows in memory space.
>
>

Since we configure the device, the PIO address must not be subject to
translation. If it is, then we must know how the translation is done.

As this is a rather crappy chip which nobody in their right mind would
use, we'll cross that bridge when/if someone decides to use it in a
system that doesn't behave like x86.

Rgds
Pierre