| Module Name:	syssrc
| Committed By:	tron
| Date:		Sun Jul 16 20:18:49 UTC 2000
| 
| Modified Files:
| 	  syssrc/sys/dev/pci: if_epic_pci.c if_tlp_pci.c
| 
| Log Message:
| If card is in power state D3 put it into power state D0 so that it will
| at least work after the next reboot.

Hmm:
  
  	if (pci_get_capability(pc, pa->pa_tag, PCI_CAP_PWRMGMT, &pmreg, 0)) {
! 		reg = pci_conf_read(pc, pa->pa_tag, pmreg + 4);
! 		switch (reg & PCI_PMCSR_STATE_MASK) {
! 		case PCI_PMCSR_STATE_D1:
! 		case PCI_PMCSR_STATE_D2:
! 			printf(": waking up from power state D%d\n%s",
! 			    reg & PCI_PMCSR_STATE_MASK, sc->sc_dev.dv_xname);
! 			pci_conf_write(pc, pa->pa_tag, pmreg + 4, 0);
! 			break;
! 		case PCI_PMCSR_STATE_D3:
  			/*
! 			 * IO and MEM are disabled. We can't enable
! 			 * the card because the BARs might be invalid.
  			 */
! 			printf(": unable to wake up from power state D3, "
! 			       "reboot required.\n");
  			pci_conf_write(pc, pa->pa_tag, pmreg + 4, 0);
+ 			return;
  		}
  	}
  

You are continuining to write to all of the bits of the PMCSR, instead of
just the PCI_PMCSR_STATE_MASK bits when you pci_conf_write(). I believe
this is wrong. There are bits there other than the Dn state that should
not be frobbed (PME, etc.), and there are read-only bits that stylistically
you should not be frobbing. Presumably you should be using

pci_conf_write(pc, pa->pa_tag, pmreg + 4,
  (reg & ~PCI_PMCSR_STATE_MASK) | PCI_PMCSR_STATE_D0);

instead.

It seems to me that the D3 state case is not quite right. You assert
that IO and MEM are disabled, but that's merely what happens on your
machine with your BIOS. Presumably that is not the case on all
machines. I assume that "Most" BIOSes will move the device to D0 on
their own and do the setup, they are not an issue. Some BIOSes may do
the PCI BAR setup but leave the device in D3. For those cases, it
should be adequate to move the device to D0 and things should just
work (disclaimed: that's how it works for the i82559, for which the
Sony VAIO bios has such a bug. I don't have a PCI PM spec and I'm not
certain absolutely what is required to switch to D0).

In any case, the comment should not asser that IO and MEM *are* disabled.
They might be disabled, and they are on your machine, but that's not true
of the general case.

I would be inclined to suggest code like:

 		case PCI_PMCSR_STATE_D3:
 			printf(": waking up from power state D%d to D0\n%s",
 			    reg & PCI_PMCSR_STATE_MASK, sc->sc_dev.dv_xname);
			pci_conf_write(pc, pa->pa_tag, pmreg + 4,
			  (reg & ~PCI_PMCSR_STATE_MASK) | PCI_PMCSR_STATE_D0);
		        if (pci_conf_read(pc, pa->pa_tag,
			    PCI_COMMAND_STATUS_REG) &
			    (PCI_COMMAND_IO_ENABLE |
			     PCI_COMMAND_MEM_ENABLE |
			     PCI_COMMAND_MASTER_ENABLE ) == 0) {
      				/*
      				 * IO and MEM are disabled. We can't enable
      				 * the card because the BARs might be invalid.
      				 */
      				printf(": unable to wake up from power state D3, "
      				       "reboot required.\n");
			}
 			return;

Though I'm not 100% it is legit.


Additionally, we now have some variant of this code in three drivers,
fxp, epic, and tlp, and it's exactly the same in epic and tlp. Perhaps
this should be abstracted out into something common in pci_subr.c.

--jhawk