[GitHub] [incubator-nuttx] jerpelea commented on pull request #1335: boards: arm: cxd56xx: enable basic snapshot camera example
jerpelea commented on pull request #1335: URL: https://github.com/apache/incubator-nuttx/pull/1335#issuecomment-656523188 @xiaoxiang781216 thanks 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #1369: arch: Initialize idle thread stack information
davids5 commented on a change in pull request #1369: URL: https://github.com/apache/incubator-nuttx/pull/1369#discussion_r452756558 ## File path: arch/arm/src/arm/arm_initialstate.c ## @@ -52,13 +52,7 @@ * Pre-processor Definitions / -/ - * Private Data - / - -/ - * Private Functions - / +#define IDLETHREAD_STACKMARGIN128 Review comment: @Ouss4 > "\tmov r1, r1, lsr #2\n" /* R1 = nwords = nbytes >> 2 */ That is the the stack size in bytes being converted to words then aligned on a word boundary. The code then jumps with a completely "colored" stack to nx_start The PR will not do the same thing. It is coloring the stack it is running on. When the stack to be colored is the stack you are running on you can take the address of a last stack var (defined volatile) in the called routine and and treat that as the end of the write. The oversize guess will not show true penetration. It will show running off the end, but you can do that with a guard block. 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1369: arch: Initialize idle thread stack information
xiaoxiang781216 commented on a change in pull request #1369: URL: https://github.com/apache/incubator-nuttx/pull/1369#discussion_r452771350 ## File path: arch/arm/src/arm/arm_initialstate.c ## @@ -52,13 +52,7 @@ * Pre-processor Definitions / -/ - * Private Data - / - -/ - * Private Functions - / +#define IDLETHREAD_STACKMARGIN128 Review comment: > When the stack to be colored is the stack you are running on you can take the address of a last stack var (defined volatile) in the called routine and and treat that as the end of the write. We need add some margin here, because the called functions(arm_stack_color) still consume some stack space. > The oversize guess will not show true penetration. It will show running off the end, but you can do that with a guard block. Any value smaller than the final stack consumption is a good margin candidate, 1/4 stack size used in new code is a reasonable value. And you can see only a few chip(13) implement the idle stack coloring, but this patch make all chipset get this feature. 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #1369: arch: Initialize idle thread stack information
davids5 commented on a change in pull request #1369: URL: https://github.com/apache/incubator-nuttx/pull/1369#discussion_r452776246 ## File path: arch/arm/src/arm/arm_initialstate.c ## @@ -52,13 +52,7 @@ * Pre-processor Definitions / -/ - * Private Data - / - -/ - * Private Functions - / +#define IDLETHREAD_STACKMARGIN128 Review comment: Why not add the "is this my stack test to arm_stack_color" and do it the right way? This comes to mind... ``` The Enemies === No Short Cuts - o Doing things the easy way instead of the correct way. ``` 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
yamt commented on pull request #1355: URL: https://github.com/apache/incubator-nuttx/pull/1355#issuecomment-656624392 @xiaoxiang781216 i think f_fsid is meant to be an ID for each individual file system, not file system type. how do you think? 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: us...@infra.apache.org
[GitHub] [incubator-nuttx-apps] yamt commented on a change in pull request #325: Change all statfs to statvfs
yamt commented on a change in pull request #325:
URL:
https://github.com/apache/incubator-nuttx-apps/pull/325#discussion_r452785786
##
File path: examples/mount/mount_main.c
##
@@ -158,40 +158,42 @@ static void show_stat(const char *path, struct stat *ps)
#endif
/
- * Name: show_statfs
+ * Name: show_statvfs
/
#ifdef TEST_USE_STATFS
-static void show_statfs(const char *path)
+static void show_statvfs(const char *path)
{
- struct statfs buf;
+ struct statvfs buf;
int ret;
- /* Try stat() against a file or directory. It should fail with
expectederror */
+ /* Try stat() against a file or directory. It should fail with
+ * expectederror
+ */
- printf("show_statfs: Try statfs(%s)\n", path);
- ret = statfs(path, &buf);
+ printf("show_statvfs: Try statvfs(%s)\n", path);
+ ret = statvfs(path, &buf);
if (ret == 0)
{
- printf("show_statfs: statfs(%s) succeeded\n", path);
- printf("\tFS Type : %0x\n", buf.f_type);
+ printf("show_statvfs: statvfs(%s) succeeded\n", path);
+ printf("\tFS Type : %0x\n", buf.f_fsid);
printf("\tBlock size: %d\n", buf.f_bsize);
printf("\tNumber of blocks : %d\n", buf.f_blocks);
printf("\tFree blocks : %d\n", buf.f_bfree);
printf("\tFree user blocks : %d\n", buf.f_bavail);
printf("\tNumber file nodes : %d\n", buf.f_files);
printf("\tFree file nodes : %d\n", buf.f_ffree);
- printf("\tFile name length : %d\n", buf.f_namelen);
+ printf("\tFile name length : %d\n", buf.f_namemax);
Review comment:
although not a fault of this patch, many of these printf formats seem
wrong.
##
File path: examples/mount/mount_main.c
##
@@ -158,40 +158,42 @@ static void show_stat(const char *path, struct stat *ps)
#endif
/
- * Name: show_statfs
+ * Name: show_statvfs
/
#ifdef TEST_USE_STATFS
-static void show_statfs(const char *path)
+static void show_statvfs(const char *path)
{
- struct statfs buf;
+ struct statvfs buf;
int ret;
- /* Try stat() against a file or directory. It should fail with
expectederror */
+ /* Try stat() against a file or directory. It should fail with
+ * expectederror
+ */
- printf("show_statfs: Try statfs(%s)\n", path);
- ret = statfs(path, &buf);
+ printf("show_statvfs: Try statvfs(%s)\n", path);
+ ret = statvfs(path, &buf);
if (ret == 0)
{
- printf("show_statfs: statfs(%s) succeeded\n", path);
- printf("\tFS Type : %0x\n", buf.f_type);
+ printf("show_statvfs: statvfs(%s) succeeded\n", path);
+ printf("\tFS Type : %0x\n", buf.f_fsid);
Review comment:
FS ID, not Type.
also, printf format needs an update for unsigned long.
##
File path: interpreters/ficl/src/nuttx.c
##
@@ -1,12 +1,58 @@
+/
+ * apps/interpreters/minibasic/basic.c
+ *
+ * Copyright (C) 2016 Gregory Nutt. All rights reserved.
+ *
+ * This file was taken from Mini Basic, versino 1.0 developed by Malcolm
+ * McLean, Leeds University. Mini Basic version 1.0 was released the
+ * Creative Commons Attribution license which, from my reading, appears to
+ * be compatible with the NuttX BSD-style license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in
+ *the documentation and/or other materials provided with the
+ *distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *used to endorse or promote products derived from this software
+ *without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, O
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1369: arch: Initialize idle thread stack information
xiaoxiang781216 commented on a change in pull request #1369: URL: https://github.com/apache/incubator-nuttx/pull/1369#discussion_r452803621 ## File path: arch/arm/src/arm/arm_initialstate.c ## @@ -52,13 +52,7 @@ * Pre-processor Definitions / -/ - * Private Data - / - -/ - * Private Functions - / +#define IDLETHREAD_STACKMARGIN128 Review comment: > Why not add the "is this my stack test to arm_stack_color" and do it the right way? So you prefer get the current stack pointer either from a local variable or xxx_getsp, but as I said before we still need some margin here, which value do you think reasonable? 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
xiaoxiang781216 commented on pull request #1355: URL: https://github.com/apache/incubator-nuttx/pull/1355#issuecomment-656648108 > @xiaoxiang781216 i think f_fsid is meant to be an ID for each individual file system, not file system type. how do you think? Yes, you are right. After more thinking, I will keep both interfaces to improve the compatibility. 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #1394: fix nxstyle warning
xiaoxiang781216 opened a new pull request #1394: URL: https://github.com/apache/incubator-nuttx/pull/1394 ## Summary ## Impact ## 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #1369: arch: Initialize idle thread stack information
davids5 commented on a change in pull request #1369:
URL: https://github.com/apache/incubator-nuttx/pull/1369#discussion_r452886847
##
File path: arch/arm/src/arm/arm_initialstate.c
##
@@ -52,13 +52,7 @@
* Pre-processor Definitions
/
-/
- * Private Data
- /
-
-/
- * Private Functions
- /
+#define IDLETHREAD_STACKMARGIN128
Review comment:
@xiaoxiang781216 I am thinking along theses lines:
Auto size it.
If the passed pointer is on the current stack. I guess (pid == 0) would
work, but generic would be better;
If you write from stack botom to current lowest marker on the stack.
```
int arm_stack_color(stack, size)
{
volatile uint32_t marker1;
register real vars...;
uint32_t nos;
volatile uint32_t marker2;
if (check stack is is mine)
{
/* make no assumptions */
nos = min( &marker2, &marker1);
size = nos - stack;
}
```
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:
us...@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1369: arch: Initialize idle thread stack information
xiaoxiang781216 commented on a change in pull request #1369:
URL: https://github.com/apache/incubator-nuttx/pull/1369#discussion_r452894490
##
File path: arch/arm/src/arm/arm_initialstate.c
##
@@ -52,13 +52,7 @@
* Pre-processor Definitions
/
-/
- * Private Data
- /
-
-/
- * Private Functions
- /
+#define IDLETHREAD_STACKMARGIN128
Review comment:
> @xiaoxiang781216 I am thinking along theses lines:
>
> Auto size it.
>
> If the passed pointer is on the current stack. I guess (pid == 0) would
work, but generic would be better;
>
> If you write from stack botom to current lowest marker on the stack.
>
> ```
> int arm_stack_color(stack, size)
> {
> volatile uint32_t marker1;
> register real vars...;
> uint32_t nos;
> volatile uint32_t marker2;
> if (check stack is is mine)
> {
> /* make no assumptions */
> nos = min( &marker2, &marker1);
>size = nos - stack;
> }
> ```
Two issues here:
1.Many arch don't have xxx_stack_color, but call memset directly.
2.The code isn't portable at least in the theroy, because the compiler
permit to move "register real vars...;" out of the marker.
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:
us...@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1355: fs/vfs: Implement statvfs and fstatvfs
xiaoxiang781216 commented on pull request #1355: URL: https://github.com/apache/incubator-nuttx/pull/1355#issuecomment-656729787 @ymat, the new patch implement statvfs/fstatvfs on top of statfs/fstatfs. Please review again. 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1315: Only build gnu_unwind_find_exidx for ARM archecture
xiaoxiang781216 commented on pull request #1315: URL: https://github.com/apache/incubator-nuttx/pull/1315#issuecomment-656730451 Can anyone approve this patch which block libcxx/uClibc++ on sim? 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: us...@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 closed pull request #325: Change all statfs to statvfs
xiaoxiang781216 closed pull request #325: URL: https://github.com/apache/incubator-nuttx-apps/pull/325 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino commented on a change in pull request #1380: arch/stm32f7: Fixes bug in tickless driver where the compare register…
antmerlino commented on a change in pull request #1380:
URL: https://github.com/apache/incubator-nuttx/pull/1380#discussion_r452987473
##
File path: arch/arm/src/stm32f7/stm32_tickless.c
##
@@ -988,22 +988,31 @@ int up_timer_start(FAR const struct timespec *ts)
}
#endif
+#ifdef CONFIG_SCHED_TICKLESS_ALARM
int up_alarm_start(FAR const struct timespec *ts)
{
uint64_t tm = ((uint64_t)ts->tv_sec * NSEC_PER_SEC + ts->tv_nsec) /
NSEC_PER_TICK;
- uint64_t counter = ((uint64_t)g_tickless.overflow << 32) |
- STM32_TIM_GETCOUNTER(g_tickless.tch);
-
- g_tickless.last_alrm = tm;
+ irqstate_t flags;
- int32_t diff = tm / NSEC_PER_TICK + counter;
+ flags = enter_critical_section();
STM32_TIM_SETCOMPARE(g_tickless.tch, CONFIG_STM32F7_TICKLESS_CHANNEL, tm);
stm32_tickless_ackint(g_tickless.channel);
stm32_tickless_enableint(CONFIG_STM32F7_TICKLESS_CHANNEL);
+ g_tickless.pending = true;
+
+ uint64_t counter = ((uint64_t)g_tickless.overflow << 32) |
+ STM32_TIM_GETCOUNTER(g_tickless.tch);
+
+ if (counter > tm)
+{
+ stm32_interval_handler();
Review comment:
Okay, my report was wrong. There was never a race condition at the
sig_timedwait level - Greg's answer cleared that up for me. I just didn't
protect for the race condition completely at the STM32 level, there was still a
very specific timing sequence that would cause it to lock up.
I've now been running stable with the latest branch.
I can't get the nxstyle line 800 error to go away. Otherwise I fixed all
other formatting issues.
This is ready for final review and then merging.
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:
us...@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino opened a new pull request #1395: drivers/eeprom/spi_xx25xx: Fixes build error.
antmerlino opened a new pull request #1395: URL: https://github.com/apache/incubator-nuttx/pull/1395 ## Summary Fixed trivial build error introduced by recent changes. ## Impact ## Testing Successfully built. 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: us...@infra.apache.org
[GitHub] [incubator-nuttx] davids5 merged pull request #1395: drivers/eeprom/spi_xx25xx: Fixes build error.
davids5 merged pull request #1395: URL: https://github.com/apache/incubator-nuttx/pull/1395 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: us...@infra.apache.org
[incubator-nuttx] branch master updated: drivers/eeprom/spi_xx25xx: Fixes build error.
This is an automated email from the ASF dual-hosted git repository.
davids5 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 731594b drivers/eeprom/spi_xx25xx: Fixes build error.
731594b is described below
commit 731594b7d2bb3004c775bd2518d5728eb54a3688
Author: Anthony Merlino
AuthorDate: Fri Jul 10 02:24:49 2020 -0400
drivers/eeprom/spi_xx25xx: Fixes build error.
---
drivers/eeprom/spi_xx25xx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/eeprom/spi_xx25xx.c b/drivers/eeprom/spi_xx25xx.c
index 88e45bc..f89c7b3 100644
--- a/drivers/eeprom/spi_xx25xx.c
+++ b/drivers/eeprom/spi_xx25xx.c
@@ -654,6 +654,7 @@ static ssize_t ee25xx_read(FAR struct file *filep, FAR char
*buffer,
{
FAR struct ee25xx_dev_s *eedev;
FAR struct inode*inode = filep->f_inode;
+ int ret;
DEBUGASSERT(inode && inode->i_private);
eedev = (FAR struct ee25xx_dev_s *)inode->i_private;
[incubator-nuttx] branch master updated (731594b -> 9dff16e)
This is an automated email from the ASF dual-hosted git repository. aguettouche pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git. from 731594b drivers/eeprom/spi_xx25xx: Fixes build error. add 9dff16e fix nxstyle warning No new revisions were added by this update. Summary of changes: arch/arm/src/cxd56xx/cxd56_usbdev.c | 1 - arch/arm/src/lc823450/lc823450_procfs_dvfs.c | 5 +- arch/arm/src/stm32/stm32_procfs_ccm.c| 7 +- arch/arm/src/stm32f7/stm32_procfs_dtcm.c | 8 +- arch/arm/src/stm32h7/stm32_procfs_dtcm.c | 6 +- fs/binfs/fs_binfs.c | 15 +- fs/cromfs/cromfs.h | 6 +- fs/littlefs/lfs_vfs.c| 5 +- fs/mount/fs_foreachmountpoint.c | 2 +- fs/mount/fs_procfs_mount.c | 15 +- fs/nfs/nfs_vfsops.c | 34 +- fs/nxffs/nxffs.h | 46 +-- fs/nxffs/nxffs_initialize.c | 28 +- fs/procfs/fs_procfscpuload.c | 11 +- fs/procfs/fs_procfscritmon.c | 5 +- fs/procfs/fs_procfsproc.c| 1 - fs/procfs/fs_procfsuptime.c | 1 - fs/procfs/fs_procfsversion.c | 3 +- fs/procfs/fs_skeleton.c | 1 - fs/smartfs/smartfs_procfs.c | 65 ++-- fs/smartfs/smartfs_smart.c | 9 +- fs/spiffs/src/spiffs_check.c | 500 +++ fs/spiffs/src/spiffs_gc.c| 224 ++-- fs/tmpfs/fs_tmpfs.h | 5 +- fs/unionfs/fs_unionfs.c | 8 +- fs/userfs/fs_userfs.c| 15 +- include/nuttx/fs/userfs.h| 10 +- libs/libc/userfs/lib_userfs.c| 82 +++-- net/procfs/net_procfs.c | 10 +- net/procfs/net_procfs_route.c| 17 +- 30 files changed, 653 insertions(+), 492 deletions(-)
[GitHub] [incubator-nuttx] Ouss4 merged pull request #1394: fix nxstyle warning
Ouss4 merged pull request #1394: URL: https://github.com/apache/incubator-nuttx/pull/1394 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: us...@infra.apache.org
