On 02/26/20 17:41, Nikita Leshenko wrote: > This series adds driver support for: > - LSI53C1030 > - SAS1068 > - SAS1068E > > These controllers are widely supported by QEMU, VirtualBox and VMWare. This > work > is part of the more general agenda of enhancing OVMF boot device support to > have > feature parity with SeaBIOS. > > We have also developed support for PVSCSI which we will submit in a separate > patch series. > > I pushed a copy of these patches to > https://github.com/nikital/edk2/tree/mptscsi > > v1->v2: > - Map() DMAed buffers > - Fixed various code convention issues > - Newer debug macros > - Updated INF version
General comments: (1) In future cover letters, please include a cumulative diffstat. "git format-patch --cover-letter" should generate one. (2) Edk2 has migrated to using SPDX license identifiers, rather than open-coded license blocks: https://bugzilla.tianocore.org/show_bug.cgi?id=1373 Furthermore, the preferred license is "BSD-2-Clause-Patent": https://spdx.org/licenses/BSD-2-Clause-Patent.html Please see "Readme.md" in the project root, or at <https://github.com/tianocore/edk2/#license-details>. (2a) Therefore please *at least* replace the open-coded BSD license blocks in the new files with SPDX tags. (2b) Furthermore, if you can consider making this contribution under "BSD-2-Clause-Patent", please change the license to that; it would be more in-line with the rest of edk2. (Otherwise, i.e. if you don't want to adopt "BSD-2-Clause-Patent", then please extend "OvmfPkg/License.txt" with the 2-clause BSD license, similarly to how "MIT" is treated there.) (2c) The "Contributed-under" tags should now be removed from the commit messages (see again TianoCore#1373). (3) Before posting the next version, please run "PatchCheck.py" on the series. For example: $ python3 BaseTools/Scripts/PatchCheck.py master..nikita-mptscsi-v2 Currently, it mainly reports "Contributed-under", but there's at least one other warning: "Line 3 of commit message is too long (78 >= 76)". Please try to fix warnings too. (4) Future versions should not be force-pushed to your repo, but pushed under a new topic branch whose name includes the version (i.e., the next version should have "v3" in the name). A topic branch, once published for review, should remain read-only. (5) Please do not include Reviewed-by and similar feedback tags that have not been given on the mailing list. For example, consider patch#4: "OvmfPkg/MptScsiDxe: Probe PCI devices and look for MptScsi" Version 1 is archived (for example) at: http://mid.mail-archive.com/20190131100724.20907-5-nikita.leshchenko@oracle.com If I remember correctly, that was the very first posting, and it already contained R-b's from Konrad, Aaron and Liran. For upstream reviews, such tags (i.e. those that originate off-list) are not useful -- they have to be given on the list, in response to the posting. (I'm pointing this out with v1 in order to show that v2 has not picked up the R-b's from the list either.) So please ask your colleagues to repost their R-b tags on the list (in response to the individual patches, or in response to the cover letter, if they approve the whole set). I'll continue with reviewing the individual patches now. Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55049): https://edk2.groups.io/g/devel/message/55049 Mute This Topic: https://groups.io/mt/71570016/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
