[parisc-linux-cvs] [PATCH] PCI MMIO mapping

Grant Grundler grundler@puffin.external.hp.com
Wed, 23 May 2001 12:48:46 -0600


Bjorn Helgaas wrote:
> I don't plan to commit this patch until after the end-of-May release, but 
> I want to get Grant's comments before he disappears, so here it is.

I didn't plan on falling off the edge of the map....just will be less
available than before. :^)

Anything I don't comment on below looks ok.

> Index: arch/parisc/kernel/inventory.c
> ===================================================================
> RCS file: /home/cvs/parisc/linux/arch/parisc/kernel/inventory.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 inventory.c
> --- inventory.c	2001/04/06 05:10:54	1.28
> +++ inventory.c	2001/05/21 18:17:50
> @@ -14,7 +14,7 @@
>  ** Debug options
>  ** DEBUG_PAT Dump details which PDC PAT provides about ranges/devices.
>  */
> -#undef DEBUG_PAT
> +#define DEBUG_PAT


We don't want to commit this right? :^)

...
> @@ -884,7 +898,6 @@ iosapic_enable_irq(void *dev, int irq)
>  
>  	/* data is initialized by fixup_irq */
>  	ASSERT(0 < vi->vi_txn_irq);
> -	ASSERT(0UL != vi->vi_txn_addr);
>  	ASSERT(0UL != vi->vi_txn_data);

Why was the ASSERT removed?
I thought txn_addr will always be non-zero for PA-RISC.
Can txn_addr be zero?

...
> @@ -1132,18 +1126,22 @@ lba_pat_resources( struct hp_device *d, 
>  
>  
>  static void
> -lba_legacy_resources( struct hp_device *d, struct lba_device *lba_dev)
> +lba_legacy_resources(struct hp_device *d, struct lba_device *lba_dev)
>  {
>  	struct resource *r;
>  	unsigned long rsize;
>  	int lba_num;
> +
>  #ifdef __LP64__
>  	/*
> +	** FIXME I can't parse this comment.
>  	** Used to sign extend instead BAR values are only 32-bit.
>  	** 64-bit BARs have the upper 32-bit's zero'd by firmware.
>  	** "Sprockets" PDC initializes for 32-bit OS.
>  	*/

Me either. Sorry. I think the comment should read:
	/*
	** Sign extend all BAR values on "legacy" platforms.
	** "Sprockets" PDC (Forte/Allegro) initializes everything for
	** for "legacy" 32-bit OS (HPUX 10.20).
	** Upper 32-bits of 64-bit BAR will be zero too.
	*/

> -	lba_dev->lmmio_base = 0xffffffff00000000UL;
> +	lba_dev->hba.mem_space_offset = 0xffffffff00000000UL;
> +#else
> +	lba_dev->hba.mem_space_offset = 0UL;
>  #endif
>  
>  	/*
> @@ -1166,10 +1164,8 @@ lba_legacy_resources( struct hp_device *
>  	r->name  = "LBA PCI LMMIO";
>  	r->flags = IORESOURCE_MEM;
>  	/* Ignore "Range Enable" bit in the BASE register */
> -	r->start = ((long) READ_REG32(d->hpa + LBA_LMMIO_BASE)) & ~1UL;
> -#ifdef __LP64__
> -	r->start |= 0xffffffff00000000UL; /* sign extend f-space */
> -#endif
> +	r->start = PCI_HOST_ADDR(HBA_DATA(lba_dev),
> +		((unsigned long) READ_REG32(d->hpa + LBA_LMMIO_BASE)) & ~1UL);

This has me slightly nervous.
Does this work on both C3000 and A500? (ie Sprockets and PAT PDCs)
If so, does it work because the value is sign extended?
Or because the READ_REG32() is cast to a 64-bit read and both PDCs
write a 64-bit value into LMMIO_BASE?


>  	rsize =  ~READ_REG32(d->hpa + LBA_LMMIO_MASK) + 1;
>  
>  	/*
> @@ -1181,7 +1177,7 @@ lba_legacy_resources( struct hp_device *
>  	r->end = r->start + rsize - 1 ;
>  
>  	/*
> -	** XXX FIXME - ignore LBA_ELMMIO_BASE for now
> +	** XXX FIXME - ignore LBA_LMMIO_BASE for now

I meant ELMMIO_BASE.

But I can't find "ELMMIO" in the Elroy ERS I have (rev 1.4, 08/23/99).
Offset 0x250 is marked "reserved". I guess one has to have HPUX source
code access to know about this. *sigh*.

elroy_cdio.h:#define ELROY_ELMMIO_BASE          0x0250

LMMIO_BASE/MASK controls the distributed MMIO range.
ELMMIO_BASE/MASK controls the directed MMIO range.

Ignoring directed ranges is a bug since it *might* prevent us from supporting
some configurations. I'm worried about attempts to reserve a MMIO resource
that isn't listed under an LBA. The drivers should get a usable value which
came out of the device BAR but reserve_resource calls should fail.
When someone figures out which configuration is broken...


>  	** "Directed" ranges are used when the "distrubuted range" isn't

s/distrubuted/distributed/

>  	** sufficient for all devices below  given LBA.  Typically devices
>  	** like graphics cards or X25 may need a directed range when the

This comment is significant for when people want to use graphics
on B1000/C3000/J5000 workstations. I'm pretty sure directed ranges are
used for fully loaded graphics on those platforms.

...
> Index: arch/parisc/kernel/pci.c
> ===================================================================
> RCS file: /home/cvs/parisc/linux/arch/parisc/kernel/pci.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 pci.c
> --- pci.c	2001/05/16 21:00:13	1.21
> +++ pci.c	2001/05/21 18:17:50
> @@ -22,7 +22,7 @@
>  
>  #ifdef CONFIG_PCI
>  
> -#undef DEBUG_RESOURCES
> +#define DEBUG_RESOURCES

This won't be committed either? :^)

...
> @@ -467,4 +464,16 @@ void pcibios_register_hba(struct pci_hba
>  	*/
>  	parisc_pci_hba[hba_count] = hba;
>  	hba->hba_num = hba_count++;
> +}
> +
> +unsigned long
> +pcibios_host_to_bus(unsigned long addr)
> +{
> +	struct pci_hba_data *hba;
> +
> +	for (hba = hba_list; NULL != hba; hba = hba->next)
> +		if (hba->mem_space.start <= addr && addr <= hba->mem_space.end)
> +			return PCI_BUS_ADDR(hba, addr);
> +
> +	panic("Invalid MMIO address 0x%lx", addr);
>  }

> Index: drivers/gsc/dino.c
> ===================================================================
> RCS file: /home/cvs/parisc/linux/drivers/gsc/dino.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 dino.c
> --- dino.c	2001/05/16 21:00:13	1.27
> +++ dino.c	2001/05/21 18:17:51
> @@ -744,6 +744,7 @@ dino_card_init(struct dino_device *dino_
>  
>  	dino_dev->ioport_addr =  0x00001000;	/* Make believe */
>  	dino_dev->mmio_addr   =  0xf0800000;	/* FIXME: Make believe */
> +	dino_dev->hba.mem_space_offset = 0;	/* CPU addrs == bus addrs */

Here, mem_space_offset should be set to -1 since the mmio_addr
is bogus too. Card-mode dino doesn't route MMIO transaction
to the PCI bus. At least not until someone writes the code to figure
out which MMIO ranges are available and programs IOADDR_EN in Dino.
PDC does this for built-in dino but not for card-mode dino.

Most of the resource mgt code is in linux. If all hppa drivers registered
regions actually used (eg PA Bus walk, CPU, etc.), card-mode dino
could allocate a chunk of F-space and use that. U2/Uturn complicates
the picture but it's still doable. I once dreamed of doing it but
it's just not interesting to me anymore.

>  	gsc_writel(0x00000000, dino_dev->hba.base_addr+DINO_DAMODE);
>  	gsc_writel(0x00222222, dino_dev->hba.base_addr+DINO_PCIROR);


...
> Index: drivers/scsi/sym53c8xx.c
> ===================================================================
> RCS file: /home/cvs/parisc/linux/drivers/scsi/sym53c8xx.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 sym53c8xx.c
> --- sym53c8xx.c	2001/01/25 00:01:46	1.11
> +++ sym53c8xx.c	2001/05/21 18:17:51
> @@ -699,11 +699,8 @@ spinlock_t sym53c8xx_lock = SPIN_LOCK_UN
>  #elif defined(__alpha__)
>  #  define pcivtobus(p)			((p) & 0xfffffffful)
>  #  define memcpy_to_pci(a, b, c)	memcpy_toio((a), (b), (c))
> -#elif defined(__hppa__) && defined(__LP64__)
> -   /* <ggg> For most PA-RISC, I think dropping upper bits would be ok for 
> now
> -    *       (VCLASS is the exception) 
> -    */
> -#  define pcivtobus(p)			((p) & 0xfffffffful)
> +#elif defined(__hppa__)
> +#  define pcivtobus(p)			pcibios_host_to_bus(p)
>  #  define memcpy_to_pci(a, b, c)	memcpy_toio((a), (b), (c))
>  #elif defined(CONFIG_PPC)
>  #  define pcivtobus(p)			phys_to_bus(p)

I'm wondering why this is replicated in the symbios driver
and not getting pulled from an include/asm file directly.
Well, it doesn't matter since pcivtobus() isn't used in 2.4.4 driver.

In fact, I'm wondering if pcibios_host_to_bus()even implements
pcivtobus() correctly.  IIRC, pcivtobus() is to convert *any* Host
virtual address into and address which can be DMA'd to by the PCI
device. This mess was replaced with Dynamic DMA mapping in 2.3 and
it's just taking the drivers a while to catch up.

Ok. The rest looked fine. Thanks!

grant

Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253