Bjorn Helgaas wrote on 2012-08-21:

On Mon, Aug 20, 2012 at 10:10 AM, Bjorn Helgaas [off-list ref]
wrote:

quoted

So I'll try pulling your branch (I'll do something about the tsi721.c
stuff myself).

I pulled this into
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git
pci/jiang-pcie-cap with the following changes:

  - Dropped some "pci_" prefixes on internal functions in access.c -
  Minor restructure of pcie_capability_read_*() - Removed export of
  pcie_capability_reg_implemented() - Reworked
  reset_intel_82599_sfp_virtfn() to check for FLR bit in DEVCAP rather
  than using pcie_capability_reg_implemented() - Split driver patches
  into one driver per patch - Fixed myri10ge_toggle_relaxed() --
  previous code returned useful value, but your patch made it always
  return zero - Use 16-bit, not 32-bit, accesses for DEVCTL, DEVCTL2
  (tsi721)
I am still concerned about reset_intel_82599_sfp_virtfn().  It looks
wrong and possibly unnecessary.  It looks wrong because it sets
PCI_EXP_DEVCTL_BCR_FLR and blindly clears all other bits in
PCI_EXP_DEVCTL.  Most of the bits are probably cleared by the FLR
anyway, but Aux Power PM Enable is RWS ("sticky"), so it is *not*
modified by FLR.  Therefore, using reset_intel_82599_sfp_virtfn() has
the probably unintended side effect of clearing Aux Power PM Enable.

It looks possibly unnecessary because the generic pcie_flr() does
essentially the same thing, so it's not clear why we need a special
case for 82599.

Yu or Dexuan, can you comment on these 82599 questions?

(Removed Yu Zhao from the To list since he has left Intel and the same
email is now used by another unrelated person...)

Hi Bjorn
I think reset_intel_82599_sfp_virtfn() is correct AND necessary.
(pcie_flr() doesn't work here)

Please note the 82599 VF is a special PCIe device that doesn't report
PCIe FLR capability though actually it does support that.
That is why we put it in quirks.c. :-)

The PCI_EXP_DEVCTL_AUX_PME bit of the 82599 VF is read-only and
always zero.

Please see 
http://www.intel.com/content/dam/doc/datasheet/82599-10-gbe-controller-datasheet.pdf
9.5 Virtual Functions Configuration Space
  Table 9.7 VF PCIe Configuration Space
  9.5.2.2.1 VF Device Control Register (0xA8; RW)

Surely I should say sorry for not putting these necessary information
in the git commit description.
Now I don't know why I forgot to do that at that time(almost 3 years ago...).

It would be great if you could help to add a comment in the code. :-)

The proposed new code is here:
http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=blob;f=driver
s/pci/
quirks.c;h=9abbf56e93fe5c98364a7dfe2b0b724047dfa4a9;hb=ef529bf0cb560
6ff5bd1422d2d2700a821d8218b#l3082

Bjorn

Thanks,
-- Dexuan