<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<html xmlns="http://www.w3.org/TR/REC-html40" 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">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<meta name="GENERATOR" content="MSHTML 8.00.6001.19298">
<!--[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-face {
        font-family: Wingdings;
}
@font-face {
        font-family: Wingdings;
}
@font-face {
        font-family: Calibri;
}
@font-face {
        font-family: Tahoma;
}
@page WordSection1 {size: 8.5in 11.0in; margin: 1.0in 1.0in 1.0in 1.0in; }
P.MsoNormal {
        MARGIN: 0in 0in 0pt; FONT-FAMILY: "Calibri","sans-serif"; FONT-SIZE: 11pt
}
LI.MsoNormal {
        MARGIN: 0in 0in 0pt; FONT-FAMILY: "Calibri","sans-serif"; FONT-SIZE: 11pt
}
DIV.MsoNormal {
        MARGIN: 0in 0in 0pt; FONT-FAMILY: "Calibri","sans-serif"; FONT-SIZE: 11pt
}
A:link {
        COLOR: blue; TEXT-DECORATION: underline; mso-style-priority: 99
}
SPAN.MsoHyperlink {
        COLOR: blue; TEXT-DECORATION: underline; mso-style-priority: 99
}
A:visited {
        COLOR: purple; TEXT-DECORATION: underline; mso-style-priority: 99
}
SPAN.MsoHyperlinkFollowed {
        COLOR: purple; TEXT-DECORATION: underline; mso-style-priority: 99
}
P.MsoAcetate {
        MARGIN: 0in 0in 0pt; FONT-FAMILY: "Tahoma","sans-serif"; FONT-SIZE: 8pt; mso-style-priority: 99; mso-style-link: "Balloon Text Char"
}
LI.MsoAcetate {
        MARGIN: 0in 0in 0pt; FONT-FAMILY: "Tahoma","sans-serif"; FONT-SIZE: 8pt; mso-style-priority: 99; mso-style-link: "Balloon Text Char"
}
DIV.MsoAcetate {
        MARGIN: 0in 0in 0pt; FONT-FAMILY: "Tahoma","sans-serif"; FONT-SIZE: 8pt; mso-style-priority: 99; mso-style-link: "Balloon Text Char"
}
P.MsoListParagraph {
        MARGIN: 0in 0in 0pt 0.5in; FONT-FAMILY: "Calibri","sans-serif"; FONT-SIZE: 11pt; mso-style-priority: 34
}
LI.MsoListParagraph {
        MARGIN: 0in 0in 0pt 0.5in; FONT-FAMILY: "Calibri","sans-serif"; FONT-SIZE: 11pt; mso-style-priority: 34
}
DIV.MsoListParagraph {
        MARGIN: 0in 0in 0pt 0.5in; FONT-FAMILY: "Calibri","sans-serif"; FONT-SIZE: 11pt; mso-style-priority: 34
}
SPAN.EmailStyle17 {
        FONT-STYLE: normal; FONT-FAMILY: "Times New Roman","serif"; COLOR: #1f497d; FONT-WEIGHT: normal; mso-style-type: personal-compose
}
SPAN.BalloonTextChar {
        FONT-FAMILY: "Tahoma","sans-serif"; mso-style-priority: 99; mso-style-link: "Balloon Text"; mso-style-name: "Balloon Text Char"
}
.MsoChpDefault {
        FONT-FAMILY: "Calibri","sans-serif"; mso-style-type: export-only
}
DIV.WordSection1 {
        page: WordSection1
}
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 dir="ltr" align="left"><font color="#0000ff" size="2" face="Arial"><span class="735163500-07092012">Hi Raymond,</span></font></div>
<div dir="ltr" align="left"><font color="#0000ff" size="2" face="Arial"><span class="735163500-07092012"></span></font> </div>
<div dir="ltr" align="left"><font color="#0000ff" size="2" face="Arial"><span class="735163500-07092012">Please see my comments in
<font color="#ff0000">red</font>...</span></font></div>
<div dir="ltr" align="left"><font color="#0000ff" size="2" face="Arial"><span class="735163500-07092012"></span></font> </div>
<div dir="ltr" align="left"><font color="#0000ff" size="2" face="Arial"><span class="735163500-07092012">Thanks,</span></font></div>
<div dir="ltr" align="left"><font color="#0000ff" size="2" face="Arial"><span class="735163500-07092012">Alex</span></font></div>
<br>
<div dir="ltr" lang="en-us" class="OutlookMessageHeader" align="left">
<hr tabindex="-1">
<font size="2" face="Tahoma"><b>From:</b> Robles, Raymond C [mailto:raymond.c.robles@intel.com]
<br>
<b>Sent:</b> Thursday, September 06, 2012 5:25 PM<br>
<b>To:</b> nvmewin@lists.openfabrics.org; Chang, Alex<br>
<b>Subject:</b> ***UNCHECKED*** Aug 28 - Patch (NVMe 1.0c Compliance and others) --- Intel Feedback<br>
</font><br>
</div>
<div></div>
<div class="WordSection1">
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">Alex,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">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-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><o:p> </o:p></span></p>
<p style="TEXT-INDENT: -0.25in; mso-list: l0 level1 lfo1" class="MsoListParagraph">
<![if !supportLists]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">-<span style="FONT: 7pt 'Times New Roman'">         
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">nvme.h:<o:p></o:p></span></p>
<![if !supportLists]>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1in; mso-list: l0 level2 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Courier New'; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">o<span style="FONT: 7pt 'Times New Roman'">   
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">ADMIN_SET_FEATURES_LBA_COMMAND_RANGE_TYPE_ENTRY Structure: GUID field (changed from ULONGLONG to UCHAR [16]) – what was the reason for this
 change?<span class="735163500-07092012"><font color="#0000ff" face="Arial"> </font></span></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1in; mso-list: l0 level2 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><span class="735163500-07092012"><font color="#ff0000" face="Arial">To match the size of GUID defined in NVMe spec, which is 16 bytes in length. If I understand it right that
 ULONGLONG is only 8-byte long.</font></span></span></p>
<![if !supportLists]>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1in; mso-list: l0 level2 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Courier New'; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">o<span style="FONT: 7pt 'Times New Roman'">   
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">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?<span class="735163500-07092012"><font color="#0000ff" face="Arial"> </font></span></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1in; mso-list: l0 level2 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><span class="735163500-07092012"><font color="#ff0000" face="Arial">No, I don't think it's backward compatible with 1.0b. The only thing I can think of as compatibility issue
 is the 0's based NUMD of Firmware Image download and Get Log Page command. In 1.0b, the spec did not indicate it clearly. Now, 1.0c clarifies it. I don't mind to add an ifdef to differenciate them.</font></span></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><o:p> </o:p></span></p>
<p style="TEXT-INDENT: -0.25in; mso-list: l0 level1 lfo1" class="MsoListParagraph">
<![if !supportLists]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">-<span style="FONT: 7pt 'Times New Roman'">         
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">nvmeInit.c:<o:p></o:p></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1in; mso-list: l0 level2 lfo1" class="MsoListParagraph">
<![if !supportLists]><span style="FONT-FAMILY: 'Courier New'; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">o<span style="FONT: 7pt 'Times New Roman'">   
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">NVMeResetAdapter:<o:p></o:p></span></p>
<![if !supportLists]>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1.5in; mso-list: l0 level3 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: Wingdings; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">§<span style="FONT: 7pt 'Times New Roman'"> 
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">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)?<span class="735163500-07092012"><font color="#0000ff" face="Arial"> </font></span></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1.5in; mso-list: l0 level3 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><font color="#ff0000"><span class="735163500-07092012"><font face="Arial">The code is checking the RDY bit to find out if the controller had already been reset. If so, there
 is no point to write 0 to EN bit of CC register again.</font> </span><o:p></o:p></font></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1in; mso-list: l0 level2 lfo1" class="MsoListParagraph">
<![if !supportLists]><span style="FONT-FAMILY: 'Courier New'; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">o<span style="FONT: 7pt 'Times New Roman'">   
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">NVMeNormalShutdown:<o:p></o:p></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1.5in; mso-list: l0 level3 lfo1" class="MsoListParagraph">
<![if !supportLists]><span style="FONT-FAMILY: Wingdings; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">§<span style="FONT: 7pt 'Times New Roman'"> 
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">Same comment as above for reset adapter (same check is performed here).<o:p></o:p></span></p>
<![if !supportLists]>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1.5in; mso-list: l0 level3 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: Wingdings; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">§<span style="FONT: 7pt 'Times New Roman'"> 
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">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.<span class="735163500-07092012"><font color="#0000ff" face="Arial"> </font></span></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1.5in; mso-list: l0 level3 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><font color="#ff0000"><span class="735163500-07092012"><font face="Arial">Per NVMe specification, when RDY bit becomes 0 due to a reset, it indicates the created queues have
 been deleted.</font> </span><o:p></o:p></font></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1in; mso-list: l0 level2 lfo1" class="MsoListParagraph">
<![if !supportLists]><span style="FONT-FAMILY: 'Courier New'; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">o<span style="FONT: 7pt 'Times New Roman'">   
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">NvmeCheckPendingCpl:<o:p></o:p></span></p>
<![if !supportLists]>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1.5in; mso-list: l0 level3 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: Wingdings; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">§<span style="FONT: 7pt 'Times New Roman'"> 
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">“unsigned int” is used for a variable declaration. We always use typedef types… should be ULONG.<span class="735163500-07092012"><font color="#0000ff" face="Arial"> </font></span></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1.5in; mso-list: l0 level3 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><font color="#ff0000"><span class="735163500-07092012"><font face="Arial">Will change it.</font> </span><o:p></o:p></font></span></p>
<![if !supportLists]>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1.5in; mso-list: l0 level3 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: Wingdings; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">§<span style="FONT: 7pt 'Times New Roman'"> 
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">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?<span class="735163500-07092012"><font color="#0000ff" face="Arial"> </font></span></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1.5in; mso-list: l0 level3 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><span class="735163500-07092012"><font color="#ff0000"><font face="Arial">I think they are for different</font> <font face="Arial">purposes. NVMeDetectPendingCmds is called
 to ensure there is no pending IO before enterring power saving modes. NVMeCheckPendingCpl is called to see if we have any pending completed entries in any one of the created completion queues to determine if we do own the INTx interrupt. In other words, pending
 IOs don't mean they had just been completed when INTx interrupt happens.</font></font></span></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><o:p> </o:p></span></p>
<p style="TEXT-INDENT: -0.25in; mso-list: l0 level1 lfo1" class="MsoListParagraph">
<![if !supportLists]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">-<span style="FONT: 7pt 'Times New Roman'">         
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">nvmeStd.c:<o:p></o:p></span></p>
<![if !supportLists]>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1in; mso-list: l0 level2 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Courier New'; COLOR: #1f497d; FONT-SIZE: 10pt"><span style="mso-list: Ignore">o<span style="FONT: 7pt 'Times New Roman'">   
</span></span></span><![endif]><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">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.<span class="735163500-07092012"><font color="#0000ff" face="Arial"> </font></span></span></p>
<p style="TEXT-INDENT: -0.25in; MARGIN-LEFT: 1in; mso-list: l0 level2 lfo1" class="MsoListParagraph">
<span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><font color="#ff0000"><span class="735163500-07092012"><font face="Arial">Will do it.</font> </span><o:p></o:p></font></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">Ray<o:p></o:p></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt"><img id="Picture_x0020_1" alt="Description: cid:image001.png@01CB3870.4BB88E70" src="cid:735163500@07092012-0577" width="54" height="39"><o:p></o:p></span></b></p>
<p class="MsoNormal"><b><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">Raymond C. Robles<o:p></o:p></span></b></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">Attached Platform Storage Software<o:p></o:p></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">Datacenter Software Division<o:p></o:p></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">Intel Corporation<o:p></o:p></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">Desk: 480.554.2600<o:p></o:p></span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Times New Roman','serif'; COLOR: #1f497d; FONT-SIZE: 10pt">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>