<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 14 (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.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Times New Roman","serif";
color:#1F497D;
font-weight:normal;
font-style:normal;}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@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="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 lfo1"><![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 lfo1">
<![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 lfo1">
<![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 lfo1"><![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 lfo1">
<![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 lfo1">
<![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 lfo1">
<![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 lfo1">
<![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 lfo1">
<![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 lfo1">
<![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 lfo1">
<![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 lfo1">
<![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 lfo1"><![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 lfo1">
<![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@01CD8C47.6B5176C0" 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>