<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=iso-8859-1"><meta name=Generator content="Microsoft Word 12 (filtered medium)"><!--[if !mso]><style>v\:* {behavior:url(#default#VML);}
o\:* {behavior:url(#default#VML);}
w\:* {behavior:url(#default#VML);}
.shape {behavior:url(#default#VML);}
</style><![endif]--><style><!--
/* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
span.EmailStyle20
{mso-style-type:personal;
font-family:"Times New Roman","serif";
color:#1F497D;
font-weight:normal;
font-style:normal;}
span.EmailStyle21
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:619729747;
mso-list-type:hybrid;
mso-list-template-ids:244464662 -340911668 67698691 67698693 67698689 67698691 67698693 67698689 67698691 67698693;}
@list l0:level1
{mso-level-start-at:0;
mso-level-number-format:bullet;
mso-level-text:-;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Times New Roman","serif";
mso-fareast-font-family:Calibri;}
@list l0:level2
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l0:level3
{mso-level-number-format:bullet;
mso-level-text:\F0A7;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;}
@list l0:level4
{mso-level-number-format:bullet;
mso-level-text:\F0B7;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Symbol;}
@list l0:level5
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l0:level6
{mso-level-number-format:bullet;
mso-level-text:\F0A7;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;}
@list l0:level7
{mso-level-number-format:bullet;
mso-level-text:\F0B7;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Symbol;}
@list l0:level8
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l0:level9
{mso-level-number-format:bullet;
mso-level-text:\F0A7;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=EN-US link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='color:#1F497D'>Hi Alex,<o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'> We were also curious about the structure change Ray mentioned. No worries on the NvmeCheckPendingCpl, as I see you are checking completion queue for newly completed entries -- our routine is for a different purpose, as it checks submission queue for commands that are pending.<o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='color:#1F497D'> Thanks,<o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='color:#1F497D'> -Rick<o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='color:#1F497D'> <o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='color:#1F497D'> <o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> nvmewin-bounces@lists.openfabrics.org [mailto:nvmewin-bounces@lists.openfabrics.org] <b>On Behalf Of </b>Robles, Raymond C<br><b>Sent:</b> Thursday, September 06, 2012 5:25 PM<br><b>To:</b> nvmewin@lists.openfabrics.org; 'Chang, Alex (Alex.Chang@idt.com)'<br><b>Subject:</b> [nvmewin] ***UNCHECKED*** Aug 28 - Patch (NVMe 1.0c Compliance and others) --- Intel Feedback<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Alex,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Here is Intel’s feedback on your patch. Let us know if you need any more info on our comments/questions.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoListParagraph style='text-indent:-.25in;mso-list:l0 level1 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><span style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>nvme.h:<o:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;mso-list:l0 level2 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:"Courier New";color:#1F497D'><span style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>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:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;mso-list:l0 level2 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:"Courier New";color:#1F497D'><span style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>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?<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoListParagraph style='text-indent:-.25in;mso-list:l0 level1 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><span style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>nvmeInit.c:<o:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;mso-list:l0 level2 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:"Courier New";color:#1F497D'><span style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>NVMeResetAdapter:<o:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;mso-list:l0 level3 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:Wingdings;color:#1F497D'><span style='mso-list:Ignore'>§<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>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:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;mso-list:l0 level2 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:"Courier New";color:#1F497D'><span style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>NVMeNormalShutdown:<o:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;mso-list:l0 level3 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:Wingdings;color:#1F497D'><span style='mso-list:Ignore'>§<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Same comment as above for reset adapter (same check is performed here).<o:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;mso-list:l0 level3 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:Wingdings;color:#1F497D'><span style='mso-list:Ignore'>§<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>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:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;mso-list:l0 level2 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:"Courier New";color:#1F497D'><span style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>NvmeCheckPendingCpl:<o:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;mso-list:l0 level3 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:Wingdings;color:#1F497D'><span style='mso-list:Ignore'>§<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>“unsigned int” is used for a variable declaration. We always use typedef types… should be ULONG.<o:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;mso-list:l0 level3 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:Wingdings;color:#1F497D'><span style='mso-list:Ignore'>§<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>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?<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoListParagraph style='text-indent:-.25in;mso-list:l0 level1 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><span style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>nvmeStd.c:<o:p></o:p></span></p><p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;mso-list:l0 level2 lfo2'><![if !supportLists]><span style='font-size:10.0pt;font-family:"Courier New";color:#1F497D'><span style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'> </span></span></span><![endif]><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>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.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Thanks,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Ray<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><b><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'><img width=54 height=39 id="Picture_x0020_1" src="cid:image001.png@01CD8C59.42BDDED0" alt="Description: cid:image001.png@01CB3870.4BB88E70"><o:p></o:p></span></b></p><p class=MsoNormal><b><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Raymond C. Robles<o:p></o:p></span></b></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Attached Platform Storage Software<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Datacenter Software Division<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Intel Corporation<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Desk: 480.554.2600<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Times New Roman","serif";color:#1F497D'>Mobile: 480.399.0645</span><span style='color:#1F497D'><o:p></o:p></span></p><p class=MsoNormal><o:p> </o:p></p></div></body></html>