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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
