Subject: Re: kern/32938: Remapping of PCMCIA IO addresses by the controller
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Sverre Froyen <sverre@viewmark.com>
List: netbsd-bugs
Date: 10/29/2006 19:20:02
The following reply was made to PR kern/32938; it has been noted by GNATS.

From: Sverre Froyen <sverre@viewmark.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/32938: Remapping of PCMCIA IO addresses by the controller
Date: Sun, 29 Oct 2006 12:14:21 -0700

 Hi,
 
 Here's my current set of patches.  New since last time is
 
 1) allowing more than one IO address space per function and
 
 2) passing back the new flag PCMCIA_IO_REMAPPED to indicate that the IO space 
 has been remapped.  The flag is then used in pcmcia.c when setting addresses.  
 The flag provides a foolproof way for the various controllers to tell 
 pcmcia.c whether or not an address space has been remapped.
 
 I believe this patch might help with PR32328 as well.
 
 Sverre
 
 Index: src/sys/dev/ic/i82365var.h
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/ic/i82365var.h,v
 retrieving revision 1.26
 diff -u -r1.26 i82365var.h
 --- src/sys/dev/ic/i82365var.h	16 Feb 2006 20:17:16 -0000	1.26
 +++ src/sys/dev/ic/i82365var.h	29 Oct 2006 18:55:43 -0000
 @@ -73,6 +73,7 @@
  	struct {
  		bus_addr_t	addr;
  		bus_size_t	size;
 +		long		offset;
  		int		width;
  	} io[PCIC_IO_WINS];
  	int	ih_irq;
 Index: src/sys/dev/pci/pccbb.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/pci/pccbb.c,v
 retrieving revision 1.136
 diff -u -r1.136 pccbb.c
 --- src/sys/dev/pci/pccbb.c	24 Oct 2006 14:16:39 -0000	1.136
 +++ src/sys/dev/pci/pccbb.c	29 Oct 2006 18:55:48 -0000
 @@ -2067,40 +2067,26 @@
  	bus_space_tag_t iot;
  	bus_space_handle_t ioh;
  	bus_addr_t mask;
 +	long *remap_offset;
  #if rbus
  	rbus_tag_t rb;
 -#endif
 -	if (align == 0) {
 -		align = size;	       /* XXX: funny??? */
 -	}
  
 -	if (start != 0) {
 -		/* XXX: assume all card decode lower 10 bits by its hardware */
 -		mask = 0x3ff;
 -		/* enforce to use only masked address */
 -		start &= mask;
 -	} else {
 -		/*
 -		 * calculate mask:
 -		 *  1. get the most significant bit of size (call it msb).
 -		 *  2. compare msb with the value of size.
 -		 *  3. if size is larger, shift msb left once.
 -		 *  4. obtain mask value to decrement msb.
 -		 */
 -		bus_size_t size_tmp = size;
 -		int shifts = 0;
 +	/*
 +	 * Set minimum alignment and mask (OK since IO
 +	 * space will be remapped by the cardbus bridge).
 +	 * XXX I think that align = 1 would be OK too except for:
 +	 * XXX size == 1 => mask = 0 which, in rbus, results
 +	 * in a full decode.  Sounds incorrect!
 +	 */
  
 -		mask = 1;
 -		while (size_tmp) {
 -			++shifts;
 -			size_tmp >>= 1;
 -		}
 -		mask = (1 << shifts);
 -		if (mask < size) {
 -			mask <<= 1;
 -		}
 -		--mask;
 -	}
 +	align = 1;
 +	while (align < size)
 +		align <<= 1;
 +	mask = align - 1;
 +#else
 +	if (align == 0)
 +		align = size;	       /* XXX: funny???, still needed? */
 +#endif
  
  	/*
  	 * Allocate some arbitrary I/O space.
 @@ -2116,6 +2102,7 @@
  	DPRINTF(("pccbb_pcmcia_io_alloc alloc port 0x%lx+0x%lx\n",
  	    (u_long) ioaddr, (u_long) size));
  #else
 +
  	if (start) {
  		ioaddr = start;
  		if (bus_space_map(iot, start, size, 0, &ioh)) {
 @@ -2135,12 +2122,21 @@
  		    (u_long) ioaddr, (u_long) size));
  	}
  #endif
 +	remap_offset = (long *) malloc(sizeof(long), M_DEVBUF, M_WAITOK);
 +	if (! remap_offset)
 +		return 1;
 +	*remap_offset = 0;
 +#if rbus
 +	*remap_offset = start - ioaddr;
 +	flags |= PCMCIA_IO_REMAPPED;
 +#endif
  
  	pcihp->iot = iot;
  	pcihp->ioh = ioh;
  	pcihp->addr = ioaddr;
  	pcihp->size = size;
  	pcihp->flags = flags;
 +	pcihp->ihandle = (void *) remap_offset;
  
  	return 0;
  }
 @@ -2164,6 +2160,9 @@
  	bus_space_handle_t ioh = pcihp->ioh;
  	bus_size_t size = pcihp->size;
  
 +	if (pcihp->ihandle)
 +		free(pcihp->ihandle, M_DEVBUF);
 +
  #if rbus
  	struct pccbb_softc *sc =
  	    (struct pccbb_softc *)((struct pcic_handle *)pch)->ph_parent;
 @@ -2203,6 +2202,7 @@
  	struct pcic_handle *ph = (struct pcic_handle *)pch;
  	bus_addr_t ioaddr = pcihp->addr + offset;
  	int i, win;
 +	long *remap_offset = (long *) pcihp->ihandle;
  #if defined CBB_DEBUG
  	static const char *width_names[] = { "dynamic", "io8", "io16" };
  #endif
 @@ -2247,6 +2247,7 @@
  	ph->io[win].addr = ioaddr;
  	ph->io[win].size = size;
  	ph->io[win].width = width;
 +	ph->io[win].offset = *remap_offset;
  
  	/* actual dirty register-value changing in the function below. */
  	pccbb_pcmcia_do_io_map(ph, win);
 @@ -2276,6 +2277,8 @@
  #define PCIC_SIA_START_HIGH 1
  #define PCIC_SIA_STOP_LOW 2
  #define PCIC_SIA_STOP_HIGH 3
 +#define PCIC_SIA_OFFSET_LOW 0x2e
 +#define PCIC_SIA_OFFSET_HIGH 0x2f
  
  	int regbase_win = 0x8 + win * 0x04;
  	u_int8_t ioctl, enable;
 @@ -2294,6 +2297,11 @@
  	Pcic_write(ph, regbase_win + PCIC_SIA_STOP_HIGH,
  	    ((ph->io[win].addr + ph->io[win].size - 1) >> 8) & 0xff);
  
 +	Pcic_write(ph, regbase_win + PCIC_SIA_OFFSET_LOW,
 +	    ph->io[win].offset & 0xff);
 +	Pcic_write(ph, regbase_win + PCIC_SIA_OFFSET_HIGH,
 +	    (ph->io[win].offset >> 8) & 0xff);
 +
  	ioctl = Pcic_read(ph, PCIC_IOCTL);
  	enable = Pcic_read(ph, PCIC_ADDRWIN_ENABLE);
  	switch (win) {
 @@ -2327,6 +2335,7 @@
  		printf("pccbb_pcmcia_do_io_map start %02x %02x, "
  		    "stop %02x %02x, ioctl %02x enable %02x\n",
  		    start_low, start_high, stop_low, stop_high, ioctl, enable);
 +		printf("pccbb_pcmcia_do_io_map: PCIC_SIA_OFFSET = %lx\n", 
 ph->io[win].offset);
  	}
  #endif
  }
 Index: src/sys/dev/pci/pccbbvar.h
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/pci/pccbbvar.h,v
 retrieving revision 1.26
 diff -u -r1.26 pccbbvar.h
 --- src/sys/dev/pci/pccbbvar.h	17 Jun 2006 17:06:52 -0000	1.26
 +++ src/sys/dev/pci/pccbbvar.h	29 Oct 2006 18:55:49 -0000
 @@ -85,6 +85,7 @@
  	struct {
  		bus_addr_t addr;
  		bus_size_t size;
 +		long offset;
  		int width;
  	} io[PCIC_IO_WINS];
  	int ih_irq;
 Index: src/sys/dev/pcmcia/pcmcia.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/pcmcia/pcmcia.c,v
 retrieving revision 1.81
 diff -u -r1.81 pcmcia.c
 --- src/sys/dev/pcmcia/pcmcia.c	24 Oct 2006 20:54:10 -0000	1.81
 +++ src/sys/dev/pcmcia/pcmcia.c	29 Oct 2006 18:55:49 -0000
 @@ -543,6 +543,9 @@
  		}
  	}
  
 +	DPRINTF(("%s: pf_mfc_iobase=0x%lx pf_mfc_iomax=0x%lx\n",
 +		 sc->dev.dv_xname, pf->pf_mfc_iobase, pf->pf_mfc_iomax));
 +
  	if (pcmcia_mfc(sc) || 1) {
  		pcmcia_ccr_write(pf, PCMCIA_CCR_IOBASE0,
  				 (pf->pf_mfc_iobase >>  0) & 0xff);
 @@ -691,27 +694,11 @@
  	if (error)
  		return (error);
  
 -	/*
 -	 * XXX in the multifunction multi-iospace-per-function case, this
 -	 * needs to cooperate with io_alloc to make sure that the spaces
 -	 * don't overlap, and that the ccr's are set correctly
 -	 */
 -
  	if (pcmcia_mfc(sc) || 1) {
 -		bus_addr_t iobase = pcihp->addr;
 -		bus_addr_t iomax = pcihp->addr + pcihp->size - 1;
  
 -		DPRINTF(("window iobase %lx iomax %lx\n", (long)iobase,
 -		    (long)iomax));
 -		if (pf->pf_mfc_iobase == 0) {
 -			pf->pf_mfc_iobase = iobase;
 -			pf->pf_mfc_iomax = iomax;
 -		} else {
 -			if (iobase < pf->pf_mfc_iobase)
 -				pf->pf_mfc_iobase = iobase;
 -			if (iomax > pf->pf_mfc_iomax)
 -				pf->pf_mfc_iomax = iomax;
 -		}
 +		DPRINTF(("window iobase %lx iomax %lx\n", (long)pcihp->addr,
 +		    (long)(pcihp->addr + pcihp->size - 1)));
 +
  		DPRINTF(("function iobase %lx iomax %lx\n",
  		    (long)pf->pf_mfc_iobase, (long)pf->pf_mfc_iomax));
  	}
 @@ -774,28 +761,118 @@
  {
  	int error = 0;
  	int n, m;
 +	bus_addr_t iobase;
 +	bus_addr_t iomax;
 +	bus_size_t align;
 +
 +	pf->pf_mfc_iobase = 1 << 26;	/* Largest possible value(?) */
 +	pf->pf_mfc_iomax = 0;
 +
 +	/*
 +	 * From Michael T. Mori & W. Dean Welder, The PCMCIA Developer's
 +	 * Guide, 3rd Edition, Sycard Tech. (1999), ISBN 0-9640342-2-0,
 +	 * p. 111. (With iomask the same as IOAddrLines.)
 +	 *
 +	 * case iomask start length   description
 +	 *   A     0      0      0     invalid
 +	 *   B     0      0      d     length d   at any d boundary +
 +	 *   C     0      c      0     invalid
 +	 *   D     0      c      d     length d   at c ++
 +	 *   E     b      0      0     length 2^b at any 2^b boundary +++
 +	 *   F     b      0      d     length d   at any 2^b boundary
 +	 *   G     b      c      0     length 2^b at c (makes no sense ++++)
 +	 *   H     b      c      d     length d   at c ++
 +	 *
 +	 * This table ignores the book's distinction between
 +	 * "Not Specified" and 0.
 +	 *
 +	 * + rounded up to nearest power of two.
 +	 *
 +	 * ++ We will set align to d (rounded up to nearest power of two) [D]
 +	 * or 2^b [H] in order to be compatible with existing controller
 +	 * code.  These cases call for adding a skew/phase parameter to the
 +	 * pcmcia_io_alloc interface [old code had skew = start & (align - 1)].
 +	 *
 +	 * +++ E cannot be valid for multiple IO spaces.
 +	 *
 +	 * ++++ as the upper end of the address range would not fit
 +	 * within iomask.
 +	 */
 +
 +	/* Determine alignment */
 +
 +	align = 1;
 +	if (cfe->iomask > 0) {
 +		/* E, F, G or H */
 +		align <<= cfe->iomask;
 +	}
 +	else {
 +		/* Determine the maximum alignment requirement */
 +		for (n = 0; n < cfe->num_iospace; n++) {
 +			bus_size_t length = cfe->iospace[n].length;
 +			if (length > 0) {
 +				/* B or D */
 +				/* XXX fix possible infinite loop */
 +				while (align < length)
 +					align <<= 1;
 +			}
 +		}
 +	}
  
  	for (n = 0; n < cfe->num_iospace; n++) {
  		bus_addr_t start = cfe->iospace[n].start;
  		bus_size_t length = cfe->iospace[n].length;
 -		bus_size_t align = cfe->iomask ? (1 << cfe->iomask) :
 -		    length;
 -		bus_size_t skew = start & (align - 1);
 -
 -		if ((start - skew) == 0 && align < 0x400) {
 -			if (skew)
 -				printf("Drats!  I need a skew!\n");
 -			start = 0;
 +
 +		if (length == 0 && start == 0 && cfe->num_iospace == 1)
 +			length = align; /* E (single IO space only) */
 +
 +		DPRINTF(("pcmcia_config_alloc: io %d start=%lx length=%lx align=%lx\n",
 +		    n, (long)start, (long)length, (long)align));
 +
 +		if (length == 0) {
 +			/* A, C or G -- invalid */
 +			error = 1;
 +			break;
  		}
  
 -		DPRINTF(("pcmcia_config_alloc: io %d start=%lx length=%lx align=%lx 
 skew=%lx\n",
 -		    n, (long)start, (long)length, (long)align, (long)skew));
 +		/* Sanity check: start + length - 1 should fit within iomask */
 +		if (cfe->iomask > 0 && start + length > 1 << cfe->iomask) {
 +			error = 1;
 +			break;
 +		}
  
  		error = pcmcia_io_alloc(pf, start, length, align,
  		    &cfe->iospace[n].handle);
  		if (error)
  			break;
 +
 +		/*
 +		 * Adjust pf->pf_mfc_iobase and pf->pf_mfc_iomax as needed.
 +		 * If the controller has set the remapped flag, use the
 +		 * card addresses, otherwise, use the addresses returned by
 +		 * the controller.
 +		 *
 +		 * XXX in the multifunction multi-iospace-per-function case,
 +		 * this needs to cooperate with io_alloc to make sure that
 +		 * the spaces don't overlap, and that the ccr's are set
 +		 * correctly.
 +		 */
 +
 +		if (cfe->iospace[n].handle.flags & PCMCIA_IO_REMAPPED) {
 +			iobase = start;
 +			iomax = start + length - 1;
 +		}
 +		else {
 +			iobase = cfe->iospace[n].handle.addr;
 +			iomax = cfe->iospace[n].handle.addr +
 +				cfe->iospace[n].handle.size - 1;
 +		}
 +		if (pf->pf_mfc_iobase > iobase)
 +			pf->pf_mfc_iobase = iobase;
 +		if (pf->pf_mfc_iomax < iomax)
 +			pf->pf_mfc_iomax = iomax;
  	}
 +
  	if (n < cfe->num_iospace) {
  		for (m = 0; m < n; m++)
  			pcmcia_io_free(pf, &cfe->iospace[m].handle);
 Index: src/sys/dev/pcmcia/pcmciavar.h
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/pcmcia/pcmciavar.h,v
 retrieving revision 1.32
 diff -u -r1.32 pcmciavar.h
 --- src/sys/dev/pcmcia/pcmciavar.h	23 Feb 2006 03:18:42 -0000	1.32
 +++ src/sys/dev/pcmcia/pcmciavar.h	29 Oct 2006 18:55:50 -0000
 @@ -49,6 +49,8 @@
  };
  
  #define	PCMCIA_IO_ALLOCATED	0x01	/* i/o space was allocated */
 +#define	PCMCIA_IO_REMAPPED	0x02	/* i/o space has been remapped, */
 +					/* card uses its own base address */
  
  /*
   * Contains information about allocated memory space.