Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[Christoph Hellwig]

On Tue, Feb 05, 2002 at 01:44:06PM -0800, David Mosberger wrote:
>   Christoph> IA64 needs to define dma64_addr_t.
> 
> Not before the driver writers understand when to use it.

Architecture maintainers are not supposed to decide whether driver
writers understand APIs.  The dma64_addr_t type is part of the PCI
DMA interface and IA64 needs to defines it.

Linus, could you please accept the below patch to define dma64_addr_t
on IA64?

	Christoph

-- 
Of course it doesn't work. We've performed a software upgrade.

Index: include/asm-ia64/types.h
===================================================================
RCS file: /vger/linux/include/asm-ia64/types.h,v
retrieving revision 1.3
diff -u -u -r1.3 types.h
--- include/asm-ia64/types.h	22 Apr 2000 00:45:18 -0000	1.3
+++ include/asm-ia64/types.h	6 Feb 2002 08:24:24 -0000
@@ -63,6 +63,7 @@
 /* DMA addresses are 64-bits wide, in general.  */
 
 typedef u64 dma_addr_t;
+typedef u64 dma64_addr_t;
 
 # endif /* __KERNEL__ */
 #endif /* !__ASSEMBLY__ */

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[David S. Miller]

   From: Christoph Hellwig <hch@caldera.de>
   Date: Wed, 6 Feb 2002 09:21:29 +0100

   On Tue, Feb 05, 2002 at 01:44:06PM -0800, David Mosberger wrote:
   >   Christoph> IA64 needs to define dma64_addr_t.
   > 
   > Not before the driver writers understand when to use it.
   
   Architecture maintainers are not supposed to decide whether driver
   writers understand APIs.  The dma64_addr_t type is part of the PCI
   DMA interface and IA64 needs to defines it.

You do have a point, but so does David.

What driver wants to get at this type and what are they using it
for?  dma_addr_t should be used by every driver I am aware of
except the clustering PCI cards I've been told about and that
driver isn't in the kernel at this time.

So who needs it? :-)

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[Christoph Hellwig]

On Wed, Feb 06, 2002 at 12:29:06AM -0800, David S. Miller wrote:
> What driver wants to get at this type and what are they using it
> for?  dma_addr_t should be used by every driver I am aware of
> except the clustering PCI cards I've been told about and that
> driver isn't in the kernel at this time.
> 
> So who needs it? :-)

The new sym53c8xx driver (sym2), and that one only if is actually is
configured for DAC-mode (SYM_CONF_DMA_ADDRESSING_MODE > 0).

	Christoph

-- 
Of course it doesn't work. We've performed a software upgrade.

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[David S. Miller]

   From: Christoph Hellwig <hch@caldera.de>
   Date: Wed, 6 Feb 2002 09:35:58 +0100

   On Wed, Feb 06, 2002 at 12:29:06AM -0800, David S. Miller wrote:
   > So who needs it? :-)
   
   The new sym53c8xx driver (sym2), and that one only if is actually is
   configured for DAC-mode (SYM_CONF_DMA_ADDRESSING_MODE > 0).

It is not using the DMA apis correctly then, it should be using
dma_addr_t which may or may not be 64-bits on a given platform.

The sym2 driver needs to be fixed.

Franks a lot,
David S. Miller
davem@redhat.com

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[Gérard Roudier]

On Wed, 6 Feb 2002, David S. Miller wrote:

>    From: Christoph Hellwig <hch@caldera.de>
>    Date: Wed, 6 Feb 2002 09:35:58 +0100
>
>    On Wed, Feb 06, 2002 at 12:29:06AM -0800, David S. Miller wrote:
>    > So who needs it? :-)
>
>    The new sym53c8xx driver (sym2), and that one only if is actually is
>    configured for DAC-mode (SYM_CONF_DMA_ADDRESSING_MODE > 0).
>
> It is not using the DMA apis correctly then, it should be using
> dma_addr_t which may or may not be 64-bits on a given platform.
>
> The sym2 driver needs to be fixed.

The code written at 't' time was using what was available at that time.
If you can let me know when Linux will be rewritten in Java, this will be
useful for me to be ready in time. :-)

--- sym_glue.c	Sun Dec 30 21:33:10 2001
+++ a.c	Tue Feb  5 22:17:30 2002
@@ -313,11 +313,7 @@
 #ifndef SYM_LINUX_DYNAMIC_DMA_MAPPING
 typedef u_long		bus_addr_t;
 #else
-#if	SYM_CONF_DMA_ADDRESSING_MODE > 0
-typedef dma64_addr_t	bus_addr_t;
-#else
 typedef dma_addr_t	bus_addr_t;
-#endif
 #endif

You may get a couple of warnings, but since I haven't hardware to check
this, I am ready to apply patches.


  Gérard.

PS:
The driver Java version is planned to be available on April the 1rst.  ;)

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[Christoph Hellwig]

On Wed, Feb 06, 2002 at 12:45:03AM -0800, David S. Miller wrote:
> It is not using the DMA apis correctly then, it should be using
> dma_addr_t which may or may not be 64-bits on a given platform.

When the sym2 driver is configured with SYM_CONF_DMA_ADDRESSING_MOD > 1
it uses DAC accessing and needs dma64_addr_t.  It doesn't use it
when using the default addressing mode.

	Christoph

-- 
Of course it doesn't work. We've performed a software upgrade.

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[Christoph Hellwig]

On Wed, Feb 06, 2002 at 06:10:42PM +0100, Christoph Hellwig wrote:
> On Wed, Feb 06, 2002 at 12:45:03AM -0800, David S. Miller wrote:
> > It is not using the DMA apis correctly then, it should be using
> > dma_addr_t which may or may not be 64-bits on a given platform.
> 
> When the sym2 driver is configured with SYM_CONF_DMA_ADDRESSING_MOD > 1
> it uses DAC accessing and needs dma64_addr_t.  It doesn't use it
> when using the default addressing mode.

Sorry, it doesn't use the _dac_ APIs.  Still I'm the opinion that either
architecture should support dma64_addr_t or we need a HAVE_PCI_DAC_API
define.

	Christoph

-- 
Of course it doesn't work. We've performed a software upgrade.

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[David Mosberger]

>>>>> On Wed, 6 Feb 2002 18:10:42 +0100, Christoph Hellwig <hch@caldera.de> said:

  Christoph> On Wed, Feb 06, 2002 at 12:45:03AM -0800, David S. Miller
  Christoph> wrote:
  >> It is not using the DMA apis correctly then, it should be using
  >> dma_addr_t which may or may not be 64-bits on a given platform.

  Christoph> When the sym2 driver is configured with
  Christoph> SYM_CONF_DMA_ADDRESSING_MOD > 1 it uses DAC accessing and
  Christoph> needs dma64_addr_t.  It doesn't use it when using the
  Christoph> default addressing mode.

The driver never uses the pci_dac* interface, hence it should not use
dma64_addr_t.  If the driver needs a 64-bit wide type, u64 will do
fine.

	--david

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[Gérard Roudier]

On Wed, 6 Feb 2002, David Mosberger wrote:

> >>>>> On Wed, 6 Feb 2002 18:10:42 +0100, Christoph Hellwig <hch@caldera.de> said:
>
>   Christoph> On Wed, Feb 06, 2002 at 12:45:03AM -0800, David S. Miller
>   Christoph> wrote:
>   >> It is not using the DMA apis correctly then, it should be using
>   >> dma_addr_t which may or may not be 64-bits on a given platform.
>
>   Christoph> When the sym2 driver is configured with
>   Christoph> SYM_CONF_DMA_ADDRESSING_MOD > 1 it uses DAC accessing and
>   Christoph> needs dma64_addr_t.  It doesn't use it when using the
>   Christoph> default addressing mode.
>
> The driver never uses the pci_dac* interface, hence it should not use
> dma64_addr_t.  If the driver needs a 64-bit wide type, u64 will do
> fine.

The driver was ready for PCI DAC months before the Linux way to do DAC was
stabilized to the current one. Given the discussion on the harmless way
the sym2 driver is using dma64_addr_t, it seems to me that this interface
may still move.

This let me think that the real problem is not the sym2 driver here, but
rather the Linux IO interfaces saga that lasts from years.

For now just replacing all occurrences (in fact only 1) of the offending
dma64 thing in sym_glue.c by the corresponding dma thing should do the
trick and close this thread as a result.

If you want to discuss about either PCI DAC under Linux or Linux-i64 arch
(that btw seems to be dead prior to have born) then, please, change the
subject of this thread.

  Gérard.

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[David S. Miller]

   From: Christoph Hellwig <hch@caldera.de>
   Date: Wed, 6 Feb 2002 18:10:42 +0100
   
   When the sym2 driver is configured with SYM_CONF_DMA_ADDRESSING_MOD > 1
   it uses DAC accessing and needs dma64_addr_t.  It doesn't use it
   when using the default addressing mode.
   
NO it damn well does not!  If the platform is NEVER GOING TO GIVE the
driver a 64-bit address (because, for example, it has IOMMU hardware),
dma_addr_t need only be 32-bits and that it how it is declared on
several platforms.

Please read the DMA API documentation.

dma64_addr_t is _ONLY_, I REPEAT _ONLY_ to be used when the driver
is making use of the following routines for it's DMA usage:

	pci_dac_page_to_dma
	pci_dac_dma_to_page
	pci_dac_dma_to_offset
	pci_dac_dma_sync_single

And NO SCSI OR NET driver should ever use these routines.

In fact, no driver in the tree right now should be using this.
The only known example that needs those interfaces are clustering
cards.  And thats it!

Everything in the tree right now should use only pci_map_single and
friends, and it should set the device DMA mask bits properly to
indicate DAC capability.  Do you see any pci_map_single, pci_map_sg,
etc. implementation working with dma64_addr_t arguments?  If so, thats
a huge bug and it must be fixed.

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[Gérard Roudier]

On Wed, 6 Feb 2002, David S. Miller wrote:

>    From: Christoph Hellwig <hch@caldera.de>
>    Date: Wed, 6 Feb 2002 18:10:42 +0100
>
>    When the sym2 driver is configured with SYM_CONF_DMA_ADDRESSING_MOD > 1
>    it uses DAC accessing and needs dma64_addr_t.  It doesn't use it
>    when using the default addressing mode.
>
> NO it damn well does not!  If the platform is NEVER GOING TO GIVE the
> driver a 64-bit address (because, for example, it has IOMMU hardware),
> dma_addr_t need only be 32-bits and that it how it is declared on
> several platforms.
>
> Please read the DMA API documentation.
>
> dma64_addr_t is _ONLY_, I REPEAT _ONLY_ to be used when the driver
> is making use of the following routines for it's DMA usage:
>
> 	pci_dac_page_to_dma
> 	pci_dac_dma_to_page
> 	pci_dac_dma_to_offset
> 	pci_dac_dma_sync_single
>
> And NO SCSI OR NET driver should ever use these routines.
>
> In fact, no driver in the tree right now should be using this.
> The only known example that needs those interfaces are clustering
> cards.  And thats it!
>
> Everything in the tree right now should use only pci_map_single and
> friends, and it should set the device DMA mask bits properly to
> indicate DAC capability.  Do you see any pci_map_single, pci_map_sg,
> etc. implementation working with dma64_addr_t arguments?  If so, thats
> a huge bug and it must be fixed.

You should calm down, in my opinion. All the burden about the PCI dma API
is your fault to you David and you David. :)
It takes a too long time for you 2 Davids to agree about this API.

Personnally, I donnot care about dma64_addr_t versus dma_addr_t being 32,
64 even 69 bits (why not, some machine with 36 bit addressing are still in
use).

Just change the topic, please. The sym2 driver has nothing to do with this
boring thread.

  Gérard.

Re: [Linux-ia64] Proper fix for sym53c8xx_2 driver and dma64_addr_t

[David Mosberger]

>>>>> On Wed, 6 Feb 2002 09:21:29 +0100, Christoph Hellwig <hch@caldera.de> said:

  Christoph> On Tue, Feb 05, 2002 at 01:44:06PM -0800, David Mosberger
  Christoph> wrote: IA64 needs to define dma64_addr_t.

  >>  Not before the driver writers understand when to use it.

  Christoph> Architecture maintainers are not supposed to decide
  Christoph> whether driver writers understand APIs.  The dma64_addr_t
  Christoph> type is part of the PCI DMA interface and IA64 needs to
  Christoph> defines it.

Then find a better way to catch errant uses of dma64_addr_t.  I have
spent too much time discussing the DMA API with Dave M to see it being
misused for no good reason.  It will take some time until there are
enough sample drivers that show proper usage of the DMA interface.
There is almost no driver that needs dma64_addr_t.  We can add it
once there is an ia64 driver that *really* needs it.

  Christoph> Linus, could you please accept the below patch to define
  Christoph> dma64_addr_t on IA64?

Christoph, if you want this type your linux distro, you can add it
with your own patch.  For the time being, I'm the ia64 linux
maintainer so I hope you can respect my decisions.

	--david