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 2021/09/17 15:03:36 UTC

[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #4570: esp32_spiflash.c: Correctly disable APP's CPU cache.

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


   ## Summary
   Disable the correct cache.
   ## Impact
   ESP32 SPI Flash driver.
   ## Testing
   esp32-devkitc:spiflash with SMP.
   


-- 
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 merged pull request #4570: esp32_spiflash.c: Correctly disable APP's CPU cache.

Posted by GitBox <gi...@apache.org>.
acassis merged pull request #4570:
URL: https://github.com/apache/incubator-nuttx/pull/4570


   


-- 
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 #4570: esp32_spiflash.c: Correctly disable APP's CPU cache.

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


   @masayuki2009 sorry I didn't see your comment earlier.  Yes, I'm working on this.  The APP CPU needs to be properly paused when the cache is disabled.


-- 
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] masayuki2009 commented on pull request #4570: esp32_spiflash.c: Correctly disable APP's CPU cache.

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #4570:
URL: https://github.com/apache/incubator-nuttx/pull/4570#issuecomment-928668262


   @Ouss4 
   
   I've just looked into the code and noticed that it manipulates the both of CPU caches.
   I think this is dangerous because the other CPU might be still running.
   So that's why you mentioned `The APP CPU needs to be properly paused when the cache is disabled.`
   
   As long as I see the code, for example, `esp32_erase()` takes `g_exclsem` and calls `esp32_erasecector()` which then calls `esp32_spiflash_opstart()` to take a critical section and disable CPU cache then issues SPI command for erase and finally calls `esp32_spiflash_opdone()` to enable CPU cache and leave the critical section. In this case, another CPU can not execute `esp32_erase()` in parallel. Also, while taking the critical section, the currently running task on the CPU would not be switched to another CPU because it took a critical section (i.e. local interrupts are also disabled) and it does not call blocking APIs. So I think cache operation for the currently running CPU is enough.
   
   By the way, I'm still not sure why we need such cache operations before and after sending SPI commands.
   In my understanding, it does not use DMA for the SPI flash. So I thought we don't need cache operations.
   
   What 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.

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] masayuki2009 commented on pull request #4570: esp32_spiflash.c: Correctly disable APP's CPU cache.

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #4570:
URL: https://github.com/apache/incubator-nuttx/pull/4570#issuecomment-928668262


   @Ouss4 
   
   I've just looked into the code and noticed that it manipulates the both of CPU caches.
   I think this is dangerous because the other CPU might be still running.
   So that's why you mentioned `The APP CPU needs to be properly paused when the cache is disabled.`
   
   As long as I see the code, for example, `esp32_erase()` takes `g_exclsem` and calls `esp32_erasecector()` which then calls `esp32_spiflash_opstart()` to take a critical section and disable CPU cache then issues SPI command for erase and finally calls `esp32_spiflash_opdone()` to enable CPU cache and leave the critical section. In this case, another CPU can not execute `esp32_erase()` in parallel. Also, while taking the critical section, the currently running task on the CPU would not be switched to another CPU because it took a critical section (i.e. local interrupts are also disabled) and it does not call blocking APIs. So I think cache operation for the currently running CPU is enough.
   
   By the way, I'm still not sure why we need such cache operations before and after sending SPI commands.
   In my understanding, it does not use DMA for the SPI flash. So I thought we don't need cache operations.
   
   What 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.

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] masayuki2009 commented on pull request #4570: esp32_spiflash.c: Correctly disable APP's CPU cache.

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #4570:
URL: https://github.com/apache/incubator-nuttx/pull/4570#issuecomment-924937553


   @Ouss4 
   
   I noticed that esp32-devkitc:wapi_smp does not work with this PR.
   
   


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