[PATCH 540] MMC_WBSD depends on ISA

[PATCH 540] MMC_WBSD depends on ISA

[Geert Uytterhoeven]

MMC_WBSD depends on ISA (needs isa_virt_to_bus())

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

--- linux-2.6.10/drivers/mmc/Kconfig	2004-12-26 10:47:03.000000000 +0100
+++ linux-m68k-2.6.10/drivers/mmc/Kconfig	2005-01-01 10:35:09.000000000 +0100
@@ -51,7 +51,7 @@ config MMC_PXA
 
 config MMC_WBSD
 	tristate "Winbond W83L51xD SD/MMC Card Interface support"
-	depends on MMC
+	depends on MMC && ISA
 	help
 	  This selects the Winbond(R) W83L51xD Secure digital and
           Multimedia card Interface.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Pierre Ossman]

Geert Uytterhoeven wrote:
> MMC_WBSD depends on ISA (needs isa_virt_to_bus())
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> --- linux-2.6.10/drivers/mmc/Kconfig	2004-12-26 10:47:03.000000000 +0100
> +++ linux-m68k-2.6.10/drivers/mmc/Kconfig	2005-01-01 10:35:09.000000000 +0100
> @@ -51,7 +51,7 @@ config MMC_PXA
>  
>  config MMC_WBSD
>  	tristate "Winbond W83L51xD SD/MMC Card Interface support"
> -	depends on MMC
> +	depends on MMC && ISA
>  	help
>  	  This selects the Winbond(R) W83L51xD Secure digital and
>            Multimedia card Interface.
> 
> Gr{oetje,eeting}s,
> 
> 						Geert
> 

Thanks. Shouldn't have missed something so obvious :)

Russell, can you fix this in your next merge?

Rgds
Pierre

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Pierre Ossman]

Pierre Ossman wrote:
> Geert Uytterhoeven wrote:
> 
>> MMC_WBSD depends on ISA (needs isa_virt_to_bus())
>>
>>
> 
> Thanks. Shouldn't have missed something so obvious :)
> 
> Russell, can you fix this in your next merge?
> 

Russell, please undo this patch. isa_virt_to_bus() is not dependent on 
CONFIG_ISA. It causes problems on x86_64 platforms which cannot enable 
ISA support.

Rgds
Pierre

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Christoph Hellwig]

On Sat, Jan 29, 2005 at 02:37:39PM +0100, Pierre Ossman wrote:
> Pierre Ossman wrote:
> >Geert Uytterhoeven wrote:
> >
> >>MMC_WBSD depends on ISA (needs isa_virt_to_bus())
> >>
> >>
> >
> >Thanks. Shouldn't have missed something so obvious :)
> >
> >Russell, can you fix this in your next merge?
> >
> 
> Russell, please undo this patch. isa_virt_to_bus() is not dependent on 
> CONFIG_ISA. It causes problems on x86_64 platforms which cannot enable 
> ISA support.
> 
> Rgds
> Pierre
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
---end quoted text---

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Russell King]

Christoph, did you mean to add anything?

On Sat, Jan 29, 2005 at 01:57:14PM +0000, Christoph Hellwig wrote:
> On Sat, Jan 29, 2005 at 02:37:39PM +0100, Pierre Ossman wrote:
> > Pierre Ossman wrote:
> > >Geert Uytterhoeven wrote:
> > >
> > >>MMC_WBSD depends on ISA (needs isa_virt_to_bus())
> > >>
> > >>
> > >
> > >Thanks. Shouldn't have missed something so obvious :)
> > >
> > >Russell, can you fix this in your next merge?
> > >
> > 
> > Russell, please undo this patch. isa_virt_to_bus() is not dependent on 
> > CONFIG_ISA. It causes problems on x86_64 platforms which cannot enable 
> > ISA support.
> > 
> > Rgds
> > Pierre
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> ---end quoted text---

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

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Christoph Hellwig]

On Sat, Jan 29, 2005 at 02:54:17PM +0000, Russell King wrote:
> Christoph, did you mean to add anything?

Yes, this somehow got lost:

-----

> > Russell, please undo this patch. isa_virt_to_bus() is not dependent on 
> > CONFIG_ISA. It causes problems on x86_64 platforms which cannot enable 
> > ISA support.
> > 

Actually it is, x86_64 just refuses to set CONFIG_ISA despite having
isa-like devices.

Either way a new driver shouldn't use isa_virt_to_bus at all but rather
use the proper DMA API and all those problems go away.

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Russell King]

On Sat, Jan 29, 2005 at 03:00:23PM +0000, Christoph Hellwig wrote:
> Either way a new driver shouldn't use isa_virt_to_bus at all but rather
> use the proper DMA API and all those problems go away.

One thing which comes up in this instance is: what struct device should
be used.

With ISA devices doing DMA, the device which actually "owns" the DMA
is the ISA DMA controller, not the device wiggling its DMA request
signal to the ISA DMA controller.

Given that, shouldn't the ISA DMA code have its own struct device to
represent the ISA DMA engine for things like the floppy driver and
other drivers using the ISA DMA engine?

To me, it makes no sense to pass the ISA device wiggling its DMA request
into the DMA API - this ISA device doesn't care whether the ISA DMA engine
can only access the first 16MB of memory or not (which is ISA DMA engine
specific), so the DMA mask on the ISA device is rather meaningless.

(This type of DMA also appears on later ARM platforms as well, hence
why I have given the whole issue some thought.)

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

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Christoph Hellwig]

On Sat, Jan 29, 2005 at 03:13:46PM +0000, Russell King wrote:
> One thing which comes up in this instance is: what struct device should
> be used.

Current convention is to use a NULL device, it's from pre-generic
DMA times were only the pci_* types existed.

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Pierre Ossman]

Christoph Hellwig wrote:

>>>Russell, please undo this patch. isa_virt_to_bus() is not dependent on 
>>>CONFIG_ISA. It causes problems on x86_64 platforms which cannot enable 
>>>ISA support.
>>>
>>>      
>>>
>
>Actually it is, x86_64 just refuses to set CONFIG_ISA despite having
>isa-like devices.
>
>Either way a new driver shouldn't use isa_virt_to_bus at all but rather
>use the proper DMA API and all those problems go away.
>
>  
>
The problem was that the DMA API didn't work for x86_64 when I wrote the 
driver. I see now that it has been fixed.
isa_virt_to_bus still works even though CONFIG_ISA is not configured. So 
I still think that the ISA dependency should be removed.
I'll move to the new API when I have the time to properly test it.

Rgds
Pierre

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Christoph Hellwig]

On Sat, Jan 29, 2005 at 04:31:16PM +0100, Pierre Ossman wrote:
> The problem was that the DMA API didn't work for x86_64 when I wrote the 
> driver. I see now that it has been fixed.
> isa_virt_to_bus still works even though CONFIG_ISA is not configured. So 

It may not exist at all.

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Pierre Ossman]

Christoph Hellwig wrote:

>On Sat, Jan 29, 2005 at 04:31:16PM +0100, Pierre Ossman wrote:
>  
>
>>The problem was that the DMA API didn't work for x86_64 when I wrote the 
>>driver. I see now that it has been fixed.
>>isa_virt_to_bus still works even though CONFIG_ISA is not configured. So 
>>    
>>
>
>It may not exist at all.
>
>  
>
For i386 and x86_64 it's defined as virt_to_phys in asm/io.h without any 
#ifdef:s protecting it.

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Christoph Hellwig]

On Sat, Jan 29, 2005 at 05:08:32PM +0100, Pierre Ossman wrote:
> For i386 and x86_64 it's defined as virt_to_phys in asm/io.h without any 
> #ifdef:s protecting it.

Not all the world is a PC

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Pierre Ossman]

Christoph Hellwig wrote:

>On Sat, Jan 29, 2005 at 05:08:32PM +0100, Pierre Ossman wrote:
>  
>
>>For i386 and x86_64 it's defined as virt_to_phys in asm/io.h without any 
>>#ifdef:s protecting it.
>>    
>>
>
>Not all the world is a PC
>
>  
>
Then the dependency should in that case be on architectures. It is 
connected similar to a floppy (which is not dependent on ISA and uses 
isa_virt_to_bus).

The point is that isa_virt_to_bus() is the method used by devices 
connected in the same way. This works on the platforms where the device 
can be found (i386 and x86_64). We can not make it dependent on ISA 
since you cannot enable ISA on all platforms where it exists (i.e. 
x86_64). Either fix that or make the driver depend on architecture the 
same way floppy does.

Using the generic DMA API might be a viable option now that x86_64 seems 
to be fixed. But it doesn't have a good track record so I'm not prepared 
to commit any changes until I have time to properly test it. There might 
still be assumptions about PCI lurking around.

Rgds
Pierre

Re: [Wbsd-devel] [PATCH 540] MMC_WBSD depends on ISA

[Geert Uytterhoeven]

On Sat, 29 Jan 2005, Pierre Ossman wrote:
> Christoph Hellwig wrote:
> > > > Russell, please undo this patch. isa_virt_to_bus() is not dependent on
> > > > CONFIG_ISA. It causes problems on x86_64 platforms which cannot enable
> > > > ISA support.
> > 
> > Actually it is, x86_64 just refuses to set CONFIG_ISA despite having
> > isa-like devices.
> > 
> > Either way a new driver shouldn't use isa_virt_to_bus at all but rather
> > use the proper DMA API and all those problems go away.
> >  
> The problem was that the DMA API didn't work for x86_64 when I wrote the
> driver. I see now that it has been fixed.
> isa_virt_to_bus still works even though CONFIG_ISA is not configured. So I
> still think that the ISA dependency should be removed.

... which makes it selectable again on all platforms that don't have ISA and
don't provide isa_virt_to_bus(), where it still breaks.

Please don't remove the dependency...

> I'll move to the new API when I have the time to properly test it.

.. but change it to e.g. `depends on ISA || X86_64', until you have moved it to
the new API.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds