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/04/07 20:11:14 UTC

[GitHub] [incubator-nuttx] Ouss4 opened a new pull request, #6006: riscv/ESP32C3: Use the common exception handler

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

   ## Summary
   ESP32-C3 was using a separate exception handler that does the same as the code we have in riscv_expcetion_common.S
   ## Impact
   ESP32-C3
   ## Testing
   ESP32-C3 defconfigs all built and run successfully.
   


-- 
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] Ouss4 commented on pull request #6006: riscv/ESP32C3: Use the common exception handler

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

   By moving the `EXCEPTION_SECTION` to chip.h, this one will break the build if merged.  It will need @pussuw fix for BL602.


-- 
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] Ouss4 commented on pull request #6006: riscv/ESP32C3: Use the common exception handler

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

   > Should we add that change into this PR? since @pussuw 's patch can't pass the CI yet.
   
   I can add parts of @pussuw commit in 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


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6006: riscv/ESP32C3: Use the common exception handler

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


-- 
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] xiaoxiang781216 commented on a diff in pull request #6006: riscv/ESP32C3: Use the common exception handler

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


##########
arch/risc-v/include/esp32c3/irq.h:
##########
@@ -229,4 +229,8 @@
 
 #define NR_IRQS  (RISCV_NIRQ_INTERRUPTS + ESP32C3_NIRQ_PERIPH + ESP32C3_NIRQ_GPIO)
 
+/* Section for exception handler. */
+
+#define EXCEPTION_SECTION .iram1

Review Comment:
   should we put to chip.h like setintstack?



-- 
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] Ouss4 commented on a diff in pull request #6006: riscv/ESP32C3: Use the common exception handler

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


##########
arch/risc-v/src/common/riscv_exception_common.S:
##########
@@ -95,7 +101,11 @@ exception_common:
   /* Offset to hartid */
 
   mv         s0, a0               /* save cause */
+#ifdef CONFIG_SMP

Review Comment:
   I've just taken a look.  Thanks for doing that!  It's in line with other archs now.
   
   Do you want me to drop this commit and wait for your PR to be merged?  My rebasing will be without conflict.



-- 
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 #6006: riscv/ESP32C3: Use the common exception handler

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


##########
arch/risc-v/src/common/riscv_exception_common.S:
##########
@@ -95,7 +101,11 @@ exception_common:
   /* Offset to hartid */
 
   mv         s0, a0               /* save cause */
+#ifdef CONFIG_SMP

Review Comment:
   No no please merge your stuff in first if you get approval, I'll do the fixing on monday so no need to wait for me. And thanks for taking a look!



-- 
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] xiaoxiang781216 commented on pull request #6006: riscv/ESP32C3: Use the common exception handler

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

   > By moving the `EXCEPTION_SECTION` to chip.h, this one will break the build if merged. It will need @pussuw fix for BL602.
   
   Look like BL602's chip.h include some header files which isn't compatible with assemble code:
   https://github.com/apache/incubator-nuttx/blob/master/arch/risc-v/src/bl602/chip.h#L28-L33
   Let's remove them?


-- 
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] Ouss4 commented on a diff in pull request #6006: riscv/ESP32C3: Use the common exception handler

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


##########
arch/risc-v/include/esp32c3/irq.h:
##########
@@ -229,4 +229,8 @@
 
 #define NR_IRQS  (RISCV_NIRQ_INTERRUPTS + ESP32C3_NIRQ_PERIPH + ESP32C3_NIRQ_GPIO)
 
+/* Section for exception handler. */
+
+#define EXCEPTION_SECTION .iram1

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.

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] xiaoxiang781216 commented on pull request #6006: riscv/ESP32C3: Use the common exception handler

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

   Should we add that change into this PR? since @pussuw 's patch can't pass the CI yet.


-- 
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 #6006: riscv/ESP32C3: Use the common exception handler

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

   > MacOS builds complaining about ARCH_CPU_COUNT, this also will be fixed with #5985.
   
   There is also a real build error:
    /Users/runner/work/incubator-nuttx/incubator-nuttx/sources/nuttx/include/stdint.h: Assembler messages:
   /Users/runner/work/incubator-nuttx/incubator-nuttx/sources/nuttx/include/stdint.h:175: Error: unrecognized opcode `typedef _int8_t int8_t'
   
   Fix for that is also in my patch `arch/risc-v/src/bl602/chip.h` is the naughty one. Needs __ASSEMBLY__ guards..


-- 
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 #6006: riscv/ESP32C3: Use the common exception handler

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


##########
arch/risc-v/src/common/riscv_exception_common.S:
##########
@@ -95,7 +101,11 @@ exception_common:
   /* Offset to hartid */
 
   mv         s0, a0               /* save cause */
+#ifdef CONFIG_SMP

Review Comment:
   No no please merge your stuff in first if you get approval, I'll do the fixing on monday so no need to wait for me



-- 
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] Ouss4 commented on pull request #6006: riscv/ESP32C3: Use the common exception handler

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

   > Needs `__ASSEMBLY__` guards..
   
   @pussuw has already fixed that.  It just required some __ASSEMBLY__ guards.


-- 
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] Ouss4 commented on pull request #6006: riscv/ESP32C3: Use the common exception handler

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

   MacOS builds complaining about ARCH_CPU_COUNT, this also will be fixed with #5985.


-- 
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] Ouss4 commented on pull request #6006: riscv/ESP32C3: Use the common exception handler

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

   BTW, I'd also like to use `riscv_vectors.S` and remove `esp32c3_vectors.S` if there is a way to do the `rept(32)` without an ugly ifdef.
   
   @pussuw there is a very small change to `riscv_exception_common` if you want to take a look.  I notice howver that the option `IRQ_NSTACKS` is only defined in MPFS Kconfig file.  This should probably go to arch/Kconfig.


-- 
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 #6006: riscv/ESP32C3: Use the common exception handler

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


##########
arch/risc-v/src/common/riscv_exception_common.S:
##########
@@ -95,7 +101,11 @@ exception_common:
   /* Offset to hartid */
 
   mv         s0, a0               /* save cause */
+#ifdef CONFIG_SMP

Review Comment:
   This will change entirely in #5985 so no worries. 



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