On 31/03/2022 17:31, Bjorn Helgaas wrote:
On Thu, Mar 31, 2022 at 10:35:39AM +0300, Shlomo Pongratz wrote:
In commit 7b94b53db34f ("PCI/P2PDMA: Add Intel Sky Lake-E Root Ports B, C, D to
the whitelist")
Andrew Maier added the Sky Lake-E additional devices
2031, 2032 and 2033 root ports to the already existing 2030 device.
The Intel devices 2030, 2031, 2032 and 2033 which are root ports A, B, C and D,
respectively and if all exist they will occupy slots 0 till 3 in that order.
Please make this a sentence.
The original code handled only the case where the devices in the whitelist are
host bridges and assumed that they will be found on slot 0.
This assumption doesn't hold for root ports so an explicit test was added to
cover this case.
Please update the subject line to match the style of previous ones.
Please wrap the commit log to fit in 80 columns (including the 4
spaces added by "git log") like previous commits.
Please figure out whether you want "Sky Lake-E" or "SkyLake-E" and use
it consistently in commit log and code comments. It seems to be
"Skylake" on intel.com, so I suggest using that.
I think that you are right and Skylake is indeed a better name,
but since Andrew Maier in his original patch used Sky Lake-E it is
better to stick with it. (SkyLake-E will be removed).
Please use imperative mood, e.g., instead of "an explicit test was
added ...," write "add a test to cover this case." Do the same in
code comments.
Bjorn
Shlomo
------------------------------------------------------------------------
*From:* Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx]
*Sent:* Thursday, March 31, 2022, 5:31 PM
*To:* Shlomo Pongratz
*Cc:* linux-pci@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx,
andrew.maier@xxxxxxxxxxxxx, logang@xxxxxxxxxxxx, bhelgaas@xxxxxxxxxx,
jgg@xxxxxxxxxx, Shlomo Pongratz
*Subject:* [PATCH V5 1/1] Intel Sky Lake-E host root ports check.
On Thu, Mar 31, 2022 at 10:35:39AM +0300, Shlomo Pongratz wrote:
In commit 7b94b53db34f ("PCI/P2PDMA: Add Intel Sky Lake-E Root Ports B, C, D to
the whitelist")
Andrew Maier added the Sky Lake-E additional devices
2031, 2032 and 2033 root ports to the already existing 2030 device.
The Intel devices 2030, 2031, 2032 and 2033 which are root ports A, B, C and D,
respectively and if all exist they will occupy slots 0 till 3 in that order.
Please make this a sentence.
The original code handled only the case where the devices in the whitelist are
host bridges and assumed that they will be found on slot 0.
This assumption doesn't hold for root ports so an explicit test was added to
cover this case.
Please update the subject line to match the style of previous ones.
Please wrap the commit log to fit in 80 columns (including the 4
spaces added by "git log") like previous commits.
Please figure out whether you want "Sky Lake-E" or "SkyLake-E" and use
it consistently in commit log and code comments. It seems to be
"Skylake" on intel.com, so I suggest using that.
Please use imperative mood, e.g., instead of "an explicit test was
added ...," write "add a test to cover this case." Do the same in
code comments.
Bjorn
