<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<small>Hi Jeremy,<br>
<br>
Thanks for the review.<br>
</small><br>
<div class="moz-cite-prefix">On 08/25/2015 06:43 AM, Jeremy Kerr
wrote:<br>
</div>
<blockquote cite="mid:55DBC152.3070304@ozlabs.org" type="cite">
<pre wrap="">Hi Neelesh,
</pre>
<blockquote type="cite">
<pre wrap="">diff --git a/external/opal-prd/hostboot-interface.h b/external/opal-prd/hostboot-interface.h
index a518f50..7ab928f 100644
--- a/external/opal-prd/hostboot-interface.h
+++ b/external/opal-prd/hostboot-interface.h
@@ -421,6 +421,41 @@ struct runtime_interfaces {
*/
int (*enable_occ_actuation)(bool i_occActivation);
+ /**
+ * @brief Apply a set of attribute overrides
+ *
+ * @param[in] pointer to binary override data
+ * @param[in] length of override data (bytes)
+ * @returns 0 on success, or return code if the command failed
+ *
+ * @platform OpenPower
+ */
+ int (*apply_attr_override)(uint8_t *i_data, size_t size);
+
+ /**
+ * @brief Send a pass-through command to HTMGT
+ *
+ * @details This is a blocking call that will send a command to
+ * HTMGT.
+ *
+ * @note If o_rspLength is returned with a non-zero value, the
+ * data at the o_rspData should be dumped to stdout in a
+ * hex dump format.
+ * @note The maximum response data returned will be 4096 bytes
+ *
+ * @param[in] i_cmdLength number of bytes in pass-thru command data
+ * @param[in] *i_cmdData pointer to pass-thru command data
+ * @param[out] *o_rspLength pointer to number of bytes returned in
+ * o_rspData
+ * @param[out] *o_rspData pointer to a 4096 byte buffer that will
+ * contain the response data from the command
+ *
+ * @returns 0 on success, or return code if the command failed
+ * @platform OpenPower
+ */
+ int (*htmgt_pass_thru)(uint16_t i_cmdLength, uint8_t *i_cmdData,
+ uint16_t *o_rspLength, uint8_t *o_rspData);
+
/* Reserve some space for future growth. */
void (*reserved[32])(void);
</pre>
</blockquote>
<pre wrap="">
Since we don't have interface version changes here, you'll need to
consume part of this reserved[] array if you add functions to the
runtime_interfaces struct.</pre>
</blockquote>
<br>
<small>Correct, I should make use of the reserve pool..</small><br>
<br>
<blockquote cite="mid:55DBC152.3070304@ozlabs.org" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c
index 8f1d6cb..9b9e213 100644
--- a/external/opal-prd/opal-prd.c
+++ b/external/opal-prd/opal-prd.c
@@ -75,6 +75,7 @@ struct opal_prd_ctx {
struct pnor pnor;
char *hbrt_file_name;
bool use_syslog;
+ bool mfg_mode;
void (*vlog)(int, const char *, va_list);
};
@@ -83,13 +84,20 @@ enum control_msg_type {
CONTROL_MSG_DISABLE_OCCS = 0x01,
CONTROL_MSG_TEMP_OCC_RESET = 0x02,
CONTROL_MSG_TEMP_OCC_ERROR = 0x03,
+ CONTROL_MSG_ATTR_OVERRIDE = 0x10,
+ CONTROL_MSG_HTMGT_PASSTHRU = 0x20,
};
</pre>
</blockquote>
<pre wrap="">
Why the gap in numbering here?</pre>
</blockquote>
<br>
<small>I was also not completely sure.. :) .. thought of grouping
them using 1st nibble,<br>
in case we add any new commands to a particular group .. <br>
Yes, it can be sequential.. will change it.<br>
</small><br>
<blockquote cite="mid:55DBC152.3070304@ozlabs.org" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">
struct control_msg {
enum control_msg_type type;
- uint64_t response;
+ int response;
+ uint32_t argc;
+ uint32_t data_len;
+ uint8_t data[];
};
+#define MAX_CONTROL_MSG_BUF 4096
+
static struct opal_prd_ctx *ctx;
static const char *opal_prd_devnode = "/dev/opal-prd";
@@ -179,6 +187,22 @@ static void pr_log_nocall(const char *name)
pr_log(LOG_WARNING, "HBRT: Call %s not provided", name);
}
+static void pr_log_stream(const uint8_t *data, uint32_t len)
</pre>
</blockquote>
<pre wrap="">
Not sure this function name matches what the function does. How about
just hexdump() ?
</pre>
<blockquote type="cite">
<pre wrap="">+ printf("\t%s htmgt-passthru <1st-byte> <2nd byte>..<nth byte>\n",
+ progname);
</pre>
</blockquote>
<pre wrap="">
Just:
printf("\t%s htmgt-passthru <bytes...>\n", progname);
</pre>
<blockquote type="cite">
<pre wrap="">+ printf("\t%s override <binary-blob>\n", progname);
</pre>
</blockquote>
<pre wrap="">
and:
printf("\t%s override <FILE>\n", progname);
(binary-blob doesn't tell the user that this is supposed to refer to a file)
</pre>
<blockquote type="cite">
<pre wrap="">+ {"mfg-mode", no_argument, NULL, 'm'},
</pre>
</blockquote>
<pre wrap="">
You listed --manufacting-mode in the commit header here. I prefer that,
but either is fine.
</pre>
<blockquote type="cite">
<pre wrap="">@@ -1483,8 +1723,29 @@ int main(int argc, char *argv[])
}
rc = send_occ_control(ctx, argv[optind + 1]);
+ break;
+ case ACTION_ATTR_OVERRIDE:
+ if (optind + 1 >= argc) {
+ pr_log(LOG_ERR, "CTRL: attribute override command "
+ "requires an argument");
+ return EXIT_FAILURE;
+ }
+
+ rc = send_attr_override(ctx, argc - optind - 1, &argv[optind + 1]);
+ break;
+ case ACTION_HTMGT_PASSTHRU:
+ if (optind + 1 >= argc) {
+ pr_log(LOG_ERR, "CTRL: htmgt passthru requires minimum "
+ "an argument");
</pre>
</blockquote>
<pre wrap="">
English nitpick: s/minimum an/at least one/</pre>
</blockquote>
<br>
<small>Will also take care of rest of the comment in next revision.<br>
<br>
Thanks,<br>
Neelesh.<br>
</small><br>
<blockquote cite="mid:55DBC152.3070304@ozlabs.org" type="cite">
<pre wrap="">
Cheers,
Jeremy
</pre>
</blockquote>
<br>
</body>
</html>