You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "GUIDINGLI (via GitHub)" <gi...@apache.org> on 2024/10/11 12:05:13 UTC
[PR] goldfish: sync with qemu do rpmsg_syslog init at goldfish [nuttx]
GUIDINGLI opened a new pull request, #14122:
URL: https://github.com/apache/nuttx/pull/14122
## Summary
goldfish: sync with qemu do rpmsg_syslog init at goldfish
## Impact
goldfish
## Testing
goldfish
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] goldfish: sync with qemu do rpmsg_syslog init at goldfish [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #14122:
URL: https://github.com/apache/nuttx/pull/14122
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] goldfish: sync with qemu do rpmsg_syslog init at goldfish [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #14122:
URL: https://github.com/apache/nuttx/pull/14122#discussion_r1797037922
##########
arch/arm/src/goldfish/goldfish_boot.c:
##########
@@ -41,6 +41,16 @@
# include <nuttx/sched_note.h>
#endif
+#include <nuttx/syslog/syslog_rpmsg.h>
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef CONFIG_SYSLOG_RPMSG
+static char g_syslog_rpmsg_buf[4096];
Review Comment:
@xiaoxiang781216 true! Thanks for warning my mistake
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] goldfish: sync with qemu do rpmsg_syslog init at goldfish [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #14122:
URL: https://github.com/apache/nuttx/pull/14122#discussion_r1796866283
##########
arch/arm/src/goldfish/goldfish_boot.c:
##########
@@ -41,6 +41,16 @@
# include <nuttx/sched_note.h>
#endif
+#include <nuttx/syslog/syslog_rpmsg.h>
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef CONFIG_SYSLOG_RPMSG
+static char g_syslog_rpmsg_buf[4096];
Review Comment:
Maybe the log buffer should be configurable, i.e. using CONFIG_SYSLOG_BUFSIZE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] goldfish: sync with qemu do rpmsg_syslog init at goldfish [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #14122:
URL: https://github.com/apache/nuttx/pull/14122#discussion_r1797007012
##########
arch/arm/src/goldfish/goldfish_boot.c:
##########
@@ -41,6 +41,16 @@
# include <nuttx/sched_note.h>
#endif
+#include <nuttx/syslog/syslog_rpmsg.h>
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef CONFIG_SYSLOG_RPMSG
+static char g_syslog_rpmsg_buf[4096];
Review Comment:
CONFIG_SYSLOG_BUFSIZE is a syslog line buffer length, not suitable here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] goldfish: sync with qemu do rpmsg_syslog init at goldfish [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #14122:
URL: https://github.com/apache/nuttx/pull/14122#issuecomment-2407292829
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
**No, this PR does not meet the NuttX requirements.**
**Missing Information:**
* **Summary:**
* Lacks a clear explanation of why the change is necessary.
* Needs to describe what "rpmsg_syslog init" is and why it needs synchronization with QEMU.
* Should specify the functional area of the code being modified.
* **Impact:**
* Simply stating "goldfish" is insufficient. Explain what "goldfish" refers to and the specific impact of the change on it.
* Address all other impact categories (user, build, hardware, documentation, security, compatibility).
* **Testing:**
* Provide details about the build host (OS, CPU, compiler) used for testing.
* Specify the exact goldfish target (board:config) used.
* Include relevant testing logs from both before and after the change, demonstrating the issue and the fix.
**Recommendations:**
1. **Expand the Summary:** Clearly articulate the problem, the proposed solution, and the code sections involved.
2. **Complete the Impact Assessment:** Address all impact categories, providing specific details where applicable.
3. **Provide Comprehensive Testing Information:** Include build host and target details and relevant testing logs.
By providing this missing information, you will create a more informative and complete PR that adheres to NuttX's contribution guidelines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] goldfish: sync with qemu do rpmsg_syslog init at goldfish [nuttx]
Posted by "CV-Bowen (via GitHub)" <gi...@apache.org>.
CV-Bowen commented on code in PR #14122:
URL: https://github.com/apache/nuttx/pull/14122#discussion_r1797001107
##########
arch/arm/src/goldfish/goldfish_boot.c:
##########
@@ -41,6 +41,16 @@
# include <nuttx/sched_note.h>
#endif
+#include <nuttx/syslog/syslog_rpmsg.h>
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef CONFIG_SYSLOG_RPMSG
+static char g_syslog_rpmsg_buf[4096];
Review Comment:
@acassis The buffer is provided by chip, should not use a system config to config this buffer size?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org