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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-