pkarashchenko commented on code in PR #10854:
URL: https://github.com/apache/nuttx/pull/10854#discussion_r1345752430


##########
arch/xtensa/src/esp32s3/esp32s3_ble_adapter.c:
##########
@@ -736,7 +773,12 @@ static void interrupt_handler_set_wrapper(int intr_num, 
void *fn, void *arg)
 
 static void interrupt_on_wrapper(int intr_num)
 {
-  up_enable_irq(intr_num + XTENSA_IRQ_FIRSTPERIPH);
+  int cpuint = intr_num;
+  int irq = esp32s3_getirq(0, cpuint);
+
+  DEBUGASSERT(esp32s3_irq_set_iram_isr(irq) == OK);

Review Comment:
   Should this be `DEBUGVERIFY`?



##########
arch/xtensa/src/esp32s3/esp32s3_ble_adapter.c:
##########
@@ -159,6 +169,22 @@ enum btdm_wakeup_src_e
   BTDM_ASYNC_WAKEUP_SRC_MAX,
 };
 
+/* Superseded semaphore definition */
+
+struct bt_sem_s
+{
+  sem_t sem;
+#ifdef CONFIG_ESP32S3_SPIFLASH
+  struct esp_semcache_s sc;
+#endif
+};
+
+struct irqstate_list_s
+{
+  FAR struct irqstate_list_s *flink;

Review Comment:
   ```suggestion
     struct irqstate_list_s *flink;
   ```
   



##########
arch/xtensa/src/esp32s3/esp32s3_ble_adapter.c:
##########
@@ -795,7 +845,15 @@ static void IRAM_ATTR interrupt_disable(void)
 
 static void IRAM_ATTR interrupt_restore(void)
 {
-  leave_critical_section(g_inter_flags);
+  struct irqstate_list_s *irqstate;
+
+  irqstate = (struct irqstate_list_s *)sq_remlast(&g_int_flags_used);
+
+  DEBUGASSERT(irqstate != NULL);
+
+  leave_critical_section(irqstate->flags);
+
+  sq_addlast((FAR sq_entry_t *)irqstate, &g_int_flags_free);

Review Comment:
   ```suggestion
     sq_addlast((sq_entry_t *)irqstate, &g_int_flags_free);
   ```
   



##########
arch/xtensa/src/esp32s3/esp32s3_ble_adapter.c:
##########
@@ -775,7 +817,15 @@ static void interrupt_off_wrapper(int intr_num)
 
 static void IRAM_ATTR interrupt_disable(void)
 {
-  g_inter_flags = enter_critical_section();
+  struct irqstate_list_s *irqstate;
+
+  irqstate = (struct irqstate_list_s *)sq_remlast(&g_int_flags_free);
+
+  DEBUGASSERT(irqstate != NULL);
+
+  irqstate->flags = enter_critical_section();
+
+  sq_addlast((FAR sq_entry_t *)irqstate, &g_int_flags_used);

Review Comment:
   ```suggestion
     sq_addlast((sq_entry_t *)irqstate, &g_int_flags_used);
   ```
   



##########
arch/xtensa/src/esp32s3/esp32s3_spiflash.c:
##########
@@ -871,7 +1109,33 @@ IRAM_ATTR int spi_flash_read(uint32_t src_addr, void 
*dest, uint32_t size)
 
 int esp32s3_spiflash_init(void)
 {
+  int cpu;
+  int ret = OK;
+
+  /* Initializes SPI flash operation lock */
+
+  nxrmutex_init(&g_flash_op_mutex);
+
+#ifdef CONFIG_SMP
+  sched_lock();
+
+  for (cpu = 0; cpu < CONFIG_SMP_NCPUS; cpu++)
+    {
+      nxsem_init(&g_disable_non_iram_isr_on_core[cpu], 0, 0);
+
+      ret = spiflash_init_spi_flash_op_block_task(cpu);
+      if (ret != OK)
+        {
+          return ret;
+        }
+    }
+
+  sched_unlock();
+#else
+    UNUSED(cpu);

Review Comment:
   ```suggestion
     UNUSED(cpu);
   ```
   



##########
arch/xtensa/src/esp32s3/esp32s3_ble_adapter.c:
##########
@@ -896,7 +960,8 @@ static int IRAM_ATTR semphr_take_from_isr_wrapper(void 
*semphr, void *hptw)
 {
   *(int *)hptw = 0;
 
-  return esp_errno_trans(nxsem_trywait(semphr));
+  DEBUGASSERT(false);

Review Comment:
   ```suggestion
     DEBUGPANIC();
   ```
   



##########
arch/xtensa/include/esp32s3/irq.h:
##########
@@ -37,6 +37,16 @@
 
 #define ESP32S3_INT_PRIO_DEF        1
 
+/* CPU interrupt flags:
+ *   These flags can be used to specify which interrupt qualities the
+ *   code calling esp32s3_setup_irq needs.
+ */
+
+#define ESP32S3_CPUINT_FLAG_LEVEL   (1 << 1) /* Level-triggered interrupt */

Review Comment:
   What bit 0 is used for?



##########
arch/xtensa/src/esp32s3/esp32s3_ble_adapter.c:
##########
@@ -2154,6 +2234,15 @@ int esp32s3_bt_controller_init(void)
   esp_bt_controller_config_t *cfg = &bt_cfg;
   bool select_src_ret;
   bool set_div_ret;
+  int i;
+
+  sq_init(&g_int_flags_free);
+  sq_init(&g_int_flags_used);
+
+  for (i = 0; i < NR_IRQSTATE_FLAGS; i++)
+    {
+      sq_addlast((FAR sq_entry_t *)&g_int_flags[i], &g_int_flags_free);

Review Comment:
   ```suggestion
         sq_addlast((sq_entry_t *)&g_int_flags[i], &g_int_flags_free);
   ```
   



##########
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;
+  FAR char *argv[2];

Review Comment:
   ```suggestion
     char *argv[2];
   ```
   



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

Reply via email to