Hi Christoph
Many Thanks for your review.
On 2021/4/3 17:50, Christoph Hellwig wrote:
+ ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
+ if (ret)
+ return;
Wouldn't it make sense to store the devcap value in the pci_dev
structure instead of reading it multiple times?
Good suggestion.
+ /* 10-Bit Tag Requester Enable in Device Control 2 Register is RsvdP for VF */
Please avoid the overly long lines.
Will fix.
+ if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT &&
+ dev->ext_10bit_tag_comp_path == 1 &&
+ (cap & PCI_EXP_DEVCAP2_10BIT_TAG_REQ)) {
+ pci_info(dev, "enabling 10-Bit Tag Requester\n");
I think that printk might become a little too noisy when lots of
devices support this capability.
Yes, How about change to pci_dbg.
+ unsigned int ext_10bit_tag_comp_path:1; /* 10-Bit Tag Completer Supported from root to here */
Another crazy long line. And why not just name this
10bit_tags?
OK, will fix. How about name this _10bit_tag or ext_10bit_tag
as C language variable names cannot start with a number.
Also a lot of this walk the upstream bridges until we hit the
root port code seems duplicatated for different capabilities. Shouldn't
we have one such walk that checks all the interesting capabilities? Or
even turn the thing around and set them on the fly while scanning
the topology?
I will investigate more about this. Thanks for your suggestion.
Thanks,
Dongdong
.
