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 2022/06/23 06:57:22 UTC

[GitHub] [incubator-nuttx] pussuw opened a new pull request, #6502: risc-v: Move common memory map to its own file from riscv_internal

pussuw opened a new pull request, #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502

   Move the linker defined symbols to a separate file, so they can be
   accessed without pulling in everything from riscv_internal.h and
   whatever it includes (e.g. syscall.h drags in a lot).
   
   ## Summary
   Split the linker defined symbols from riscv_internal.h. Main reason is to allow the xxx_memorymap.h files to include this instead of pulling in the entire riscv_internal.h file (and what it includes)
   ## Impact
   Minor refactoring
   ## Testing
   icicle
   


-- 
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


[GitHub] [incubator-nuttx] pussuw commented on pull request #6502: risc-v: Move common memory map to its own file from riscv_internal

Posted by GitBox <gi...@apache.org>.
pussuw commented on PR #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502#issuecomment-1167182707

   The fix for the build error is here:
   https://github.com/apache/incubator-nuttx/pull/6530


-- 
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


[GitHub] [incubator-nuttx] pkarashchenko merged pull request #6502: risc-v: Move common memory map to its own file from riscv_internal

Posted by GitBox <gi...@apache.org>.
pkarashchenko merged PR #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502


-- 
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


[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6502: risc-v: Move common memory map to its own file from riscv_internal

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502#discussion_r905022361


##########
arch/risc-v/src/common/riscv_common_memorymap.h:
##########
@@ -0,0 +1,103 @@
+/****************************************************************************
+ * arch/risc-v/src/common/riscv_common_memorymap.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+#define __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define _START_TEXT  &_stext
+#define _END_TEXT    &_etext
+#define _START_BSS   &_sbss
+#define _END_BSS     &_ebss
+#define _DATA_INIT   &_eronly
+#define _START_DATA  &_sdata
+#define _END_DATA    &_edata
+#define _START_TDATA &_stdata
+#define _END_TDATA   &_etdata
+#define _START_TBSS  &_stbss
+#define _END_TBSS    &_etbss
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+#ifndef __ASSEMBLY__
+EXTERN uintptr_t g_idle_topstack;
+
+/* Address of per-cpu idle stack base */
+
+EXTERN const uint8_t * const g_cpu_basestack[CONFIG_SMP_NCPUS];

Review Comment:
   `CONFIG_SMP_NCPUS` is defined on `<sys/types.h>` when `SMP` is not selected:
   https://github.com/apache/incubator-nuttx/blob/master/include/sys/types.h#L63-L65



-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6502: risc-v: Move common memory map to its own file from riscv_internal

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502#discussion_r907065957


##########
arch/risc-v/src/common/riscv_common_memorymap.h:
##########
@@ -0,0 +1,105 @@
+/****************************************************************************
+ * arch/risc-v/src/common/riscv_common_memorymap.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+#define __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define _START_TEXT  &_stext
+#define _END_TEXT    &_etext
+#define _START_BSS   &_sbss
+#define _END_BSS     &_ebss
+#define _DATA_INIT   &_eronly
+#define _START_DATA  &_sdata
+#define _END_DATA    &_edata
+#define _START_TDATA &_stdata
+#define _END_TDATA   &_etdata
+#define _START_TBSS  &_stbss
+#define _END_TBSS    &_etbss
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+#ifndef __ASSEMBLY__
+EXTERN uintptr_t g_idle_topstack;
+
+/* Address of per-cpu idle stack base */
+
+EXTERN const uint8_t * const g_cpu_basestack[CONFIG_SMP_NCPUS];
+
+/* Address of the saved user stack pointer */
+
+#if CONFIG_ARCH_INTERRUPTSTACK > 15
+EXTERN uint32_t g_intstackalloc; /* Allocated stack base */
+EXTERN uint32_t g_intstacktop;   /* Initial top of interrupt stack */
+#endif
+
+/* These 'addresses' of these values are setup by the linker script.  They
+ * are not actual uint32_t storage locations! They are only used meaningfully
+ * in the following way:
+ *
+ *  - The linker script defines, for example, the symbol_sdata.
+ *  - The declaration extern uint32_t _sdata; makes C happy.  C will believe
+ *    that the value _sdata is the address of a uint32_t variable _data (it
+ *    is not!).
+ *  - We can recover the linker value then by simply taking the address of
+ *    of _data.  like:  uint32_t *pdata = &_sdata;
+ */
+
+EXTERN uint32_t _stext;           /* Start of .text */
+EXTERN uint32_t _etext;           /* End_1 of .text + .rodata */

Review Comment:
   in general I think that text is also read only and can be marked as `const` as well as `_eronly`, but since it is refactoring only, let's keep it as is



-- 
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


[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6502: risc-v: Move common memory map to its own file from riscv_internal

Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502#discussion_r907237841


##########
arch/risc-v/src/common/riscv_common_memorymap.h:
##########
@@ -0,0 +1,105 @@
+/****************************************************************************
+ * arch/risc-v/src/common/riscv_common_memorymap.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+#define __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define _START_TEXT  &_stext
+#define _END_TEXT    &_etext
+#define _START_BSS   &_sbss
+#define _END_BSS     &_ebss
+#define _DATA_INIT   &_eronly
+#define _START_DATA  &_sdata
+#define _END_DATA    &_edata
+#define _START_TDATA &_stdata
+#define _END_TDATA   &_etdata
+#define _START_TBSS  &_stbss
+#define _END_TBSS    &_etbss
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+#ifndef __ASSEMBLY__
+EXTERN uintptr_t g_idle_topstack;
+
+/* Address of per-cpu idle stack base */
+
+EXTERN const uint8_t * const g_cpu_basestack[CONFIG_SMP_NCPUS];
+
+/* Address of the saved user stack pointer */
+
+#if CONFIG_ARCH_INTERRUPTSTACK > 15
+EXTERN uint32_t g_intstackalloc; /* Allocated stack base */
+EXTERN uint32_t g_intstacktop;   /* Initial top of interrupt stack */
+#endif
+
+/* These 'addresses' of these values are setup by the linker script.  They
+ * are not actual uint32_t storage locations! They are only used meaningfully
+ * in the following way:
+ *
+ *  - The linker script defines, for example, the symbol_sdata.
+ *  - The declaration extern uint32_t _sdata; makes C happy.  C will believe
+ *    that the value _sdata is the address of a uint32_t variable _data (it
+ *    is not!).
+ *  - We can recover the linker value then by simply taking the address of
+ *    of _data.  like:  uint32_t *pdata = &_sdata;
+ */
+
+EXTERN uint32_t _stext;           /* Start of .text */
+EXTERN uint32_t _etext;           /* End_1 of .text + .rodata */

Review Comment:
   I'm not sure why _eronly has const in this scope. These are not actual memory locations (variables), just a way to get the linker to fill in a relocation symbol



-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6502: risc-v: Move common memory map to its own file from riscv_internal

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502#discussion_r907258126


##########
arch/risc-v/src/common/riscv_common_memorymap.h:
##########
@@ -0,0 +1,105 @@
+/****************************************************************************
+ * arch/risc-v/src/common/riscv_common_memorymap.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+#define __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define _START_TEXT  &_stext
+#define _END_TEXT    &_etext
+#define _START_BSS   &_sbss
+#define _END_BSS     &_ebss
+#define _DATA_INIT   &_eronly
+#define _START_DATA  &_sdata
+#define _END_DATA    &_edata
+#define _START_TDATA &_stdata
+#define _END_TDATA   &_etdata
+#define _START_TBSS  &_stbss
+#define _END_TBSS    &_etbss
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+#ifndef __ASSEMBLY__
+EXTERN uintptr_t g_idle_topstack;
+
+/* Address of per-cpu idle stack base */
+
+EXTERN const uint8_t * const g_cpu_basestack[CONFIG_SMP_NCPUS];
+
+/* Address of the saved user stack pointer */
+
+#if CONFIG_ARCH_INTERRUPTSTACK > 15
+EXTERN uint32_t g_intstackalloc; /* Allocated stack base */
+EXTERN uint32_t g_intstacktop;   /* Initial top of interrupt stack */
+#endif
+
+/* These 'addresses' of these values are setup by the linker script.  They
+ * are not actual uint32_t storage locations! They are only used meaningfully
+ * in the following way:
+ *
+ *  - The linker script defines, for example, the symbol_sdata.
+ *  - The declaration extern uint32_t _sdata; makes C happy.  C will believe
+ *    that the value _sdata is the address of a uint32_t variable _data (it
+ *    is not!).
+ *  - We can recover the linker value then by simply taking the address of
+ *    of _data.  like:  uint32_t *pdata = &_sdata;
+ */
+
+EXTERN uint32_t _stext;           /* Start of .text */
+EXTERN uint32_t _etext;           /* End_1 of .text + .rodata */

Review Comment:
   Yes, maybe we can remove const.
   The const will prevent accidental writing by assignment, but not more.



-- 
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


[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6502: risc-v: Move common memory map to its own file from riscv_internal

Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502#discussion_r905706945


##########
arch/risc-v/src/common/riscv_common_memorymap.h:
##########
@@ -0,0 +1,103 @@
+/****************************************************************************
+ * arch/risc-v/src/common/riscv_common_memorymap.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+#define __ARCH_RISC_V_SRC_COMMON_RISCV_COMMON_MEMORYMAP_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define _START_TEXT  &_stext
+#define _END_TEXT    &_etext
+#define _START_BSS   &_sbss
+#define _END_BSS     &_ebss
+#define _DATA_INIT   &_eronly
+#define _START_DATA  &_sdata
+#define _END_DATA    &_edata
+#define _START_TDATA &_stdata
+#define _END_TDATA   &_etdata
+#define _START_TBSS  &_stbss
+#define _END_TBSS    &_etbss
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+#ifndef __ASSEMBLY__
+EXTERN uintptr_t g_idle_topstack;
+
+/* Address of per-cpu idle stack base */
+
+EXTERN const uint8_t * const g_cpu_basestack[CONFIG_SMP_NCPUS];

Review Comment:
   Thanks for pointing this out, will fix



-- 
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


[GitHub] [incubator-nuttx] acassis commented on pull request #6502: risc-v: Move common memory map to its own file from riscv_internal

Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502#issuecomment-1164404337

   > Don't think the error is my doing
   > 
   > ```
   > curl: (28) Failed to connect to www.phyplusinc.com port 80: Connection timed out
   > cp: cannot stat 'libphy62xxble.a': No such file or directory
   > make[1]: *** [chip/Make.defs:82: .buildlib] Error 1
   > make[1]: Target 'context' not remade because of errors.
   > make: *** [tools/Unix.mk:425: arch/arm/src/.context] Error 2
   > make: Target 'all' not remade because of errors.
   > ```
   
   It is not! Maybe we need to have some mirror to avoid these sites disappear and we miss the closed source/proprietary libraries...


-- 
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


[GitHub] [incubator-nuttx] pussuw commented on pull request #6502: risc-v: Move common memory map to its own file from riscv_internal

Posted by GitBox <gi...@apache.org>.
pussuw commented on PR #6502:
URL: https://github.com/apache/incubator-nuttx/pull/6502#issuecomment-1164107759

   Don't think the error is my doing
   
   ```
   curl: (28) Failed to connect to www.phyplusinc.com port 80: Connection timed out
   cp: cannot stat 'libphy62xxble.a': No such file or directory
   make[1]: *** [chip/Make.defs:82: .buildlib] Error 1
   make[1]: Target 'context' not remade because of errors.
   make: *** [tools/Unix.mk:425: arch/arm/src/.context] Error 2
   make: Target 'all' not remade because of errors.
   ```


-- 
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