[PATCH] [MMC] Fix base address configuration in wbsd

[PATCH] [MMC] Fix base address configuration in wbsd

[Pierre Ossman]

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 <drzeus@drzeus.cx>
---

 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;
 }

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

[Russell King]

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

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

[Pierre Ossman]

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

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

[Russell King]

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

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

[Pierre Ossman]

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

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

[Ben Dooks]

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 (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

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

[Pierre Ossman]

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.

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

[Alan Cox]

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

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

[Pierre Ossman]

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

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

[Randy.Dunlap]

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

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

[Pierre Ossman]

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