Messages in this thread | | | Date | Fri, 1 Apr 2022 19:11:57 +0300 | Subject | Re: [PATCH V5 1/1] Intel Sky Lake-E host root ports check. | From | Shlomo Pongratz <> |
| |
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@kernel.org] *Sent:* Thursday, March 31, 2022, 5:31 PM *To:* Shlomo Pongratz *Cc:* linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, andrew.maier@eideticom.com, logang@deltatee.com, bhelgaas@google.com, jgg@nvidia.com, 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
| |