pkarashchenko commented on code in PR #10854:
URL: https://github.com/apache/nuttx/pull/10854#discussion_r1346328585
##########
arch/xtensa/src/esp32s3/esp32s3_irq.c:
##########
@@ -785,6 +873,15 @@ int esp32s3_setup_irq(int cpu, int periphid, int priority,
int type)
intmap[cpuint] = CPUINT_ASSIGN(periphid + XTENSA_IRQ_FIRSTPERIPH);
g_irqmap[irq] = IRQ_MKMAP(cpu, cpuint);
+ if ((flags & ESP32S3_CPUINT_FLAG_IRAM) != 0)
+ {
+ esp32s3_irq_set_iram_isr(irq);
+ }
+ else
+ {
+ esp32s3_irq_unset_iram_isr(irq);
Review Comment:
From my experience usually set/clear notation is used, but set/unset is also
fine
##########
arch/xtensa/src/esp32s3/esp32s3_spiflash.c:
##########
@@ -858,6 +924,178 @@ IRAM_ATTR int spi_flash_read(uint32_t src_addr, void
*dest, uint32_t size)
}
#endif /* CONFIG_ESP32S3_SPI_FLASH_DONT_USE_ROM_CODE */
+/****************************************************************************
+ * Name: spi_flash_disable_cache
+ *
+ * Description:
+ * Disable the I/D cache of a CPU core and save its status value on
+ * s_flash_op_cache_state.
+ *
+ * Input Parameters:
+ * cpuid - The CPU core whose cache will be disabled.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+static void spi_flash_disable_cache(uint32_t cpuid)
+{
+ s_flash_op_cache_state[cpuid] = cache_suspend_icache() << 16;
+ s_flash_op_cache_state[cpuid] |= cache_suspend_dcache();
+}
+
+/****************************************************************************
+ * Name: spi_flash_restore_cache
+ *
+ * Description:
+ * Restore the I/D cache of a CPU core using the saved status value on
+ * s_flash_op_cache_state.
+ *
+ * Input Parameters:
+ * cpuid - The CPU core whose cache will be restored.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+static void spi_flash_restore_cache(uint32_t cpuid)
+{
+ cache_resume_dcache(s_flash_op_cache_state[cpuid] & 0xffff);
+ cache_resume_icache(s_flash_op_cache_state[cpuid] >> 16);
+}
+
+#ifdef CONFIG_SMP
+
+/****************************************************************************
+ * Name: spi_flash_op_block_task
+ *
+ * Description:
+ * Disable the non-IRAM interrupts on the other core (the one that isn't
+ * handling the SPI flash operation) and notify that the SPI flash
+ * operation can start. Wait on a busy loop until it's finished and then
+ * reenable the non-IRAM interrups.
+ *
+ * Input Parameters:
+ * argc - Not used.
+ * argv - Not used.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. A negated errno value is returned to
+ * indicate the nature of any failure.
+ *
+ ****************************************************************************/
+
+static int spi_flash_op_block_task(int argc, char *argv[])
+{
+ struct tcb_s *tcb = this_task();
+ int cpu = up_cpu_index();
+
+ for (; ; )
+ {
+ DEBUGASSERT((1 << cpu) & tcb->affinity);
+ /* Wait for a SPI flash operation to take place and this (the other
+ * core) being asked to disable its non-IRAM interrupts.
+ */
+
+ nxsem_wait(&g_disable_non_iram_isr_on_core[cpu]);
+
+ sched_lock();
+
+ esp32s3_irq_noniram_disable();
+
+ /* g_flash_op_complete flag is cleared on *this* CPU, otherwise the
+ * other CPU may reset the flag back to false before this task has a
+ * chance to check it (if it's preempted by an ISR taking non-trivial
+ * amount of time).
+ */
+
+ g_flash_op_complete = false;
+ g_flash_op_can_start = true;
+ while (!g_flash_op_complete)
+ {
+ /* Busy loop here and wait for the other CPU to finish the SPI
+ * flash operation.
+ */
+ }
+
+ /* Flash operation is complete, re-enable cache */
+
+ spi_flash_restore_cache(cpu);
+
+ /* Restore interrupts that aren't located in IRAM */
+
+ esp32s3_irq_noniram_enable();
+
+ sched_unlock();
+ }
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: spiflash_init_spi_flash_op_block_task
+ *
+ * Description:
+ * Starts a kernel thread that waits for a semaphore indicating that a SPI
+ * flash operation is going to take place in the other CPU. It disables
+ * non-IRAM interrupts, indicates to the other core that the SPI flash
+ * operation can start and waits for it to be finished in a busy loop.
+ *
+ * Input Parameters:
+ * cpu - The CPU core that will run the created task to wait on a busy
+ * loop while the SPI flash operation finishes
+ *
+ * Returned Value:
+ * 0 (OK) on success; A negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int spiflash_init_spi_flash_op_block_task(int cpu)
+{
+ int pid;
+ int ret;
+ char *argv[2];
+ char arg1[32];
+ cpu_set_t cpuset;
+
+ snprintf(arg1, sizeof(arg1), "%p", &cpu);
+ argv[0] = arg1;
+ argv[1] = NULL;
+
+ pid = kthread_create("spiflash_op",
+ SCHED_PRIORITY_MAX,
+ CONFIG_ESP32S3_SPIFLASH_OP_TASK_STACKSIZE,
+ spi_flash_op_block_task,
+ argv);
+ if (pid > 0)
+ {
+ if (cpu < CONFIG_SMP_NCPUS)
+ {
+ CPU_ZERO(&cpuset);
+ CPU_SET(cpu, &cpuset);
+ ret = nxsched_set_affinity(pid, sizeof(cpuset), &cpuset);
+ if (ret < 0)
+ {
+ return ret;
+ }
+ }
+ }
+ else
+ {
+ ret = ret == OK ? -EPERM : ret;
Review Comment:
Seems like accessing not initialized variable 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-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
