[nvmewin] ***UNCHECKED*** Aug 28 - Patch (NVMe 1.0c Compliance and others) --- Intel Feedback

[Robles, Raymond C]

Alex,

Here is Intel's feedback on your patch.  Let us know if you need any more info on our comments/questions.


-          nvme.h:

o    ADMIN_SET_FEATURES_LBA_COMMAND_RANGE_TYPE_ENTRY Structure: GUID field (changed from ULONGLONG to UCHAR [16]) - what was the reason for this change?

o    General Comment: With the 1.0c changes, will the driver be backward compatible with 1.0b? If not, do we need a mechanism to do so or have you thought about what we should be doing in this case?  Did you attempt any testing of this?


-          nvmeInit.c:

o    NVMeResetAdapter:

§  What is the use case for having a check for RDY already being 0 (we can never have nested resets so it would seem this would never be the case but not totally sure)?

o    NVMeNormalShutdown:

§  Same comment as above for reset adapter (same check is performed here).

§  The comment on line 2469 states that the code is waiting for all queues to be deleted, but really you are just checking to see that the RDY bit has been set to 0 indicating the transition of the EN bit from 1 to 0.

o    NvmeCheckPendingCpl:

§  "unsigned int" is used for a variable declaration. We always use typedef types... should be ULONG.

§  General Comment: There is already a function to detect if commands are pending... NVMeDetectPendingCmds in nvmeIo.c. This was done as part of the S3/S4 work that Rick/Arpit (LSI) did. Did you take a look at this function and see if it was similar to your new function? Was there something specific that you needed differently than what was already coded in the existing function?


-          nvmeStd.c:

o    Line 1657: Paul removed all support for CHATHAM in a previous patch (but left in CHATHAM2 support). Please remove the CHATHAM check from the code.

Thanks,
Ray

[Description: cid:image001.png at 01CB3870.4BB88E70]
Raymond C. Robles
Attached Platform Storage Software
Datacenter Software Division
Intel Corporation
Desk: 480.554.2600
Mobile: 480.399.0645

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20120907/e1012169/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.png
Type: image/png
Size: 1756 bytes
Desc: image001.png
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20120907/e1012169/attachment.png>