You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/08/05 17:49:40 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #1520: Move note driver from drivers/syslog to drivers/note
xiaoxiang781216 opened a new pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520
## Summary
It's better to put the note transport layer into a common folder.
## Impact
No functionality change.
## Testing
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-671056782
> I think this change is wrong. At least it conflicts with my intention to integration USB FIFO hardware. For my desired usage, I must be able to go directly to to the memory mapped FIFO with no intervening buffering in memory. That would destroy that solution. I must have the capability to enable the instrumentation is NO buffering. This PR incorrectly removes that capability. Please restore the ability to avoid the buffering.
@patacongo you have the NO buffering capability, actually this is the main target of this patch want to achieve. With this patch:
1.sched_note.c implment sched_note_* function which packet the various arguments to the common struct note_*.
2.The transport layer only need implement one function `void note_add(FAR const uint8_t *note, uint8_t notelen);`
Step 1 directly pass the formated result to the transport layer without any internal buffer.
Step 2 decide how to handle the formated result(buffer in RAM or send to USB).
The benefit is that the transport layer don't need implement the bunch of schd_note_* function and then could focus on how to transfer the data efficiently in note_add.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-675572208
@patacongo could you take a look more carefully the patch is a path to your final goal. @YuuichiNakamura and my patch is wating this one to merge:
1.Move https://github.com/apache/incubator-nuttx/blob/master/arch/sim/src/sim/up_schednote.c to drivers/note
2.Rename note_driver.c and up_schednote.c to note_ram.c and note_syslog.c
3.Add the note filter control
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-671056782
> I think this change is wrong. At least it conflicts with my intention to integration USB FIFO hardware. For my desired usage, I must be able to go directly to to the memory mapped FIFO with no intervening buffering in memory. That would destroy that solution. I must have the capability to enable the instrumentation is NO buffering. This PR incorrectly removes that capability. Please restore the ability to avoid the buffering.
@patacongo you have the NO buffering capability, actually this is the main target of this patch want to achieve. With this patch:
1.sched_note.c implment sched_note_* function which packet the various arguments to the common struct note_.
2.The transport layer only need implement one function `void note_add(FAR const uint8_t *note, uint8_t notelen);`
Step 1 directly pass the formated result to the transport layer without any internal buffer.
Step 2 decide how to handle the formated result(buffer in RAM or send to USB).
The benefit is that the transport layer don't need implement the bunch of schd_note_* function and then could focus on how to transfer the data efficiently in note_add.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-683581640
@patacongo could you point out what your requirement doesn't be considered? USB transport layer can be implemented by one function now:
```
void note_add(FAR const uint8_t *note, uint8_t notelen);
```
which is much simpler than to implement a bunch of functions:
```
void sched_note_start(FAR struct tcb_s *tcb);
void sched_note_stop(FAR struct tcb_s *tcb);
void sched_note_suspend(FAR struct tcb_s *tcb);
void sched_note_resume(FAR struct tcb_s *tcb);
#ifdef CONFIG_SMP
void sched_note_cpu_start(FAR struct tcb_s *tcb, int cpu);
void sched_note_cpu_started(FAR struct tcb_s *tcb);
void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
void sched_note_cpu_paused(FAR struct tcb_s *tcb);
void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_PREEMPTION
void sched_note_premption(FAR struct tcb_s *tcb, bool locked);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_CSECTION
void sched_note_csection(FAR struct tcb_s *tcb, bool enter);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
void sched_note_spinlock(FAR struct tcb_s *tcb,
FAR volatile void *spinlock);
void sched_note_spinlocked(FAR struct tcb_s *tcb,
FAR volatile void *spinlock);
void sched_note_spinunlock(FAR struct tcb_s *tcb,
FAR volatile void *spinlock);
void sched_note_spinabort(FAR struct tcb_s *tcb,
FAR volatile void *spinlock);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
void sched_note_syscall_enter(int nr, int argc, ...);
void sched_note_syscall_leave(int nr, uintptr_t result);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
#endif
```
The implementation of ```sche_note_*``` in sched/sched/sched_note.c just pacakage the various arguments into ```struct note_*``` and pass ```note_add``` directly without any buffering. It's decided by the transport layer(RAM, USB etc.) whether to buffer the data and how to transfer the data.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-669338737
@YuuichiNakamura
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] liuguo09 merged pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
liuguo09 merged pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-683808356
> The header file for this driver is still at include/nuttx/syslog/note_driver.h.
> This should also be moved to include/nuttx/note/note_driver.h.
good point, done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-669336510
See the discussion here: https://github.com/apache/incubator-nuttx/pull/1377
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#discussion_r480140656
##########
File path: include/nuttx/sched_note.h
##########
@@ -349,47 +338,24 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
#endif
/****************************************************************************
- * Name: sched_note_get
+ * Name: note_add
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Add the variable length note to the transport layer
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * note - The note buffer
+ * notelne - The buffer length
Review comment:
Done.
##########
File path: drivers/note/Kconfig
##########
@@ -0,0 +1,44 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menu "Note Driver Support"
+
+config DRIVER_NOTE
+ bool "Scheduler instrumentation driver"
+ default n
+ depends on !SCHED_INSTRUMENTATION_CSECTION && (!SCHED_INSTRUMENTATION_SPINLOCK || !SMP)
+ ---help---
+ If this option is selected, then in-memory buffering logic is
+ enabled to capture scheduler instrumentation data. This has
+ the advantage that (1) the platform logic does not have to provide
+ the sched_note_* interfaces described for the previous settings.
+ Instead, the buffering logic catches all of these. It encodes
+ timestamps the scheduler note and adds the note to an in-memory,
+ circular buffer. And (2) buffering the scheduler instrumentation
+ data (versus performing some output operation) minimizes the impact
+ of the instrumentation on the behavior of the system. If the in-memory
+ buffer becomes full, then older notes are overwritten by newer notes.
+
+ A charac driver is provided which can be used by an application
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-686655589
Does anyone have more comment? If not, we will merge tomorrow to unblock @YuuichiNakamura 's work.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-683581640
@patacongo could you point out what your requirement doesn't be considered? USB transport can be implemented by one function:
```
void note_add(FAR const uint8_t *note, uint8_t notelen);
```
which is much simpler than to implment a bunch of functions:
```
void sched_note_start(FAR struct tcb_s *tcb);
void sched_note_stop(FAR struct tcb_s *tcb);
void sched_note_suspend(FAR struct tcb_s *tcb);
void sched_note_resume(FAR struct tcb_s *tcb);
#ifdef CONFIG_SMP
void sched_note_cpu_start(FAR struct tcb_s *tcb, int cpu);
void sched_note_cpu_started(FAR struct tcb_s *tcb);
void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
void sched_note_cpu_paused(FAR struct tcb_s *tcb);
void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_PREEMPTION
void sched_note_premption(FAR struct tcb_s *tcb, bool locked);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_CSECTION
void sched_note_csection(FAR struct tcb_s *tcb, bool enter);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
void sched_note_spinlock(FAR struct tcb_s *tcb,
FAR volatile void *spinlock);
void sched_note_spinlocked(FAR struct tcb_s *tcb,
FAR volatile void *spinlock);
void sched_note_spinunlock(FAR struct tcb_s *tcb,
FAR volatile void *spinlock);
void sched_note_spinabort(FAR struct tcb_s *tcb,
FAR volatile void *spinlock);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
void sched_note_syscall_enter(int nr, int argc, ...);
void sched_note_syscall_leave(int nr, uintptr_t result);
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
#endif
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-683799234
The header file for this driver is still at include/nuttx/syslog/note_driver.h.
This should also be moved to include/nuttx/note/note_driver.h.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-671056782
> I think this change is wrong. At least it conflicts with my intention to integration USB FIFO hardware. For my desired usage, I must be able to go directly to to the memory mapped FIFO with no intervening buffering in memory. That would destroy that solution. I must have the capability to enable the instrumentation is NO buffering. This PR incorrectly removes that capability. Please restore the ability to avoid the buffering.
@patacongo you have the NO buffering capability, actually this is the main target of this patch want to achieve. With this patch:
1.sched_note.c implment `sched_note_* function` which packet the various arguments to the common `struct note_*`.
2.The transport layer only need implement one function `void note_add(FAR const uint8_t *note, uint8_t notelen);`
Step 1 directly pass the formated result to the transport layer without any internal buffer.
Step 2 decide how to handle the formated result(buffer in RAM or send to USB).
The benefit is that the transport layer don't need implement the bunch of `schd_note_* function` and then could focus on how to transfer the data efficiently in `note_add`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#discussion_r480086830
##########
File path: drivers/note/Kconfig
##########
@@ -0,0 +1,44 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menu "Note Driver Support"
+
+config DRIVER_NOTE
+ bool "Scheduler instrumentation driver"
+ default n
+ depends on !SCHED_INSTRUMENTATION_CSECTION && (!SCHED_INSTRUMENTATION_SPINLOCK || !SMP)
+ ---help---
+ If this option is selected, then in-memory buffering logic is
+ enabled to capture scheduler instrumentation data. This has
+ the advantage that (1) the platform logic does not have to provide
+ the sched_note_* interfaces described for the previous settings.
+ Instead, the buffering logic catches all of these. It encodes
+ timestamps the scheduler note and adds the note to an in-memory,
+ circular buffer. And (2) buffering the scheduler instrumentation
+ data (versus performing some output operation) minimizes the impact
+ of the instrumentation on the behavior of the system. If the in-memory
+ buffer becomes full, then older notes are overwritten by newer notes.
+
+ A charac driver is provided which can be used by an application
Review comment:
```suggestion
A character driver is provided which can be used by an application
```
##########
File path: include/nuttx/sched_note.h
##########
@@ -349,47 +338,24 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
#endif
/****************************************************************************
- * Name: sched_note_get
+ * Name: note_add
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Add the variable length note to the transport layer
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * note - The note buffer
+ * notelne - The buffer length
Review comment:
```suggestion
* notelen - The buffer length
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1520: Move note driver from drivers/syslog to drivers/note
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1520:
URL: https://github.com/apache/incubator-nuttx/pull/1520#issuecomment-671056782
> I think this change is wrong. At least it conflicts with my intention to integration USB FIFO hardware. For my desired usage, I must be able to go directly to to the memory mapped FIFO with no intervening buffering in memory. That would destroy that solution. I must have the capability to enable the instrumentation is NO buffering. This PR incorrectly removes that capability. Please restore the ability to avoid the buffering.
@patacongo you have the NO buffering capability, actually this is the main target of this patch want to achieve. With this patch:
1.sched_note.c implment sched_note_* function which packet the various arguments to the common struct note_.
2.The transport layer only need implement one function `void note_add(FAR const uint8_t *note, uint8_t notelen);`
Step 1 directly pass the formated result to the transport layer without any internal buffer.
Step 2 decide how to handle the formated result(buffer in RAM or send to USB).
The benefit is that the transport layer don't need implement the bunch of schd_note_* function and then could focus on to transfer the data effiently in note_add.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org