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: 05/11/2006 18:50: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: Thu, 11 May 2006 12:47:21 -0600

 Updated patch for src/sys/dev/pcmcia/pvmcia.c.  The new patch contains 
 comments from "The PCMCIA Developer's Guide" about the correct interpretation 
 of the iomask, start, and length CIS parameters, new code matching the 
 comments, and a couple of bug fixes.  Please verify that the removal of the 
 tests around line 715 does not cause compatibility problems (these tests were 
 removed in order to make the debug printout, just below the tests, appear 
 correct, it had no effect on the working of my modem card).
 
 Index: src/sys/dev/pcmcia/pcmcia.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/pcmcia/pcmcia.c,v
 retrieving revision 1.79
 diff -u -r1.79 pcmcia.c
 --- src/sys/dev/pcmcia/pcmcia.c	29 Mar 2006 06:00:46 -0000	1.79
 +++ src/sys/dev/pcmcia/pcmcia.c	11 May 2006 17:53:24 -0000
 @@ -548,6 +548,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);
 @@ -711,12 +714,8 @@
  		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(("function iobase %lx iomax %lx\n",
  		    (long)pf->pf_mfc_iobase, (long)pf->pf_mfc_iomax));
  	}
 @@ -780,26 +779,87 @@
  	int error = 0;
  	int n, m;
  
 +	pf->pf_mfc_iobase = 0;
 +	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.
 +	 * ++ as the upper end of the address range would not fit
 +	 * within iomask.
 +	 */
 +
  	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;
 +		bus_size_t align = 1;
 +
 +		/* D and H need no processing */
 +
 +		if (start == 0) {
 +			if (cfe->iomask > 0) {
 +				/* E or F */
 +				align = (1 << cfe->iomask);
 +				if (length == 0)
 +					length = align; /* E */
 +			}
 +			else if (length > 0) {
 +				/* B */
 +				/* align equals 1 at this point */
 +				/* XXX fix possible infinite loop */
 +				while (align < length)
 +					align <<= 1;
 +			}
 +		}
 +
 +		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;
 +		/*
 +		 * Set pf_mfc_iobase equal to start. This flags the case
 +		 * where the card specifies a fixed IO base address and the
 +		 * controller remaps that address.
 +		 * XXX Need to rethink this in the case of multiple IO
 +		 * spaces per function. Fail for now.
 +		 */
 +		if (pf->pf_mfc_iobase != 0) {
 +			DPRINTF(("pcmcia_config_alloc: cannot (yet) remap multiple IO spaces\n"));
 +			error = 1;
 +			break;
 +		}
 +		pf->pf_mfc_iobase = start;
 +		pf->pf_mfc_iomax = start + length - 1;
  	}
  	if (n < cfe->num_iospace) {
  		for (m = 0; m < n; m++)