drivers: i2s_nrfx: Fix write race condition

There is inherent race condition between i2s_nrfx_write() and I2S
interrupt handler because I2S operates independently from the rest
of the system. If software takes too long to supply next TX pointer
then nRF I2S peripheral will simply resupply the previous buffer.

The race window is rather short. The failed race executes as follows:
  1. i2s_nrfx_write() checks state and loads next_tx_buffer_needed
  2. I2S interrupt handler executes and calls data_handler() which
     notices empty TX queue and therefore sets next_tx_buffer_needed
  3. i2s_nrfx_write() continues with the queue TX path (because the
     next_tx_buffer_needed was false when it was accessed)

If next i2s_nrfx_write() executes before next I2S interrupt:
  4a. i2s_nrfx_write() notices next_tx_buffer_needed is true and
      supplies the buffer directly to I2S peripheral. Previously queued
      buffer will remain in the queue until the just supplied buffer
      starts transmitting. Effectively swapping whole I2S block leads to
      clearly audible artifacts under normal circumstances.

If next I2S interrupt executes before next i2s_nrfx_write():
  4b. data_handler() notices that buffer was reused and stops despite
      having a buffer available in TX queue

Modify i2s_nrfx_write() to always queue the TX pointer first and only
supply the buffer to nrfx if the queue was empty when interrupt handler
executed. This prevents both the out-of-order TX and premature stop.

Fixes: #63730

Signed-off-by: Tomasz Moń <tomasz.mon@nordicsemi.no>

drivers/i2s/i2s_nrfx.c

@@ -583,6 +583,7 @@ static int i2s_nrfx_write(const struct device *dev,
 			  void *mem_block, size_t size)
 {
 	struct i2s_nrfx_drv_data *drv_data = dev->data;
+	int ret;
 
 	if (!drv_data->tx_configured) {
 		LOG_ERR("Device is not configured");
@@ -601,26 +602,41 @@ static int i2s_nrfx_write(const struct device *dev,
 		return -EIO;
 	}
 
+	ret = k_msgq_put(&drv_data->tx_queue,
+			 &mem_block,
+			 SYS_TIMEOUT_MS(drv_data->tx.cfg.timeout));
+	if (ret < 0) {
+		return ret;
+	}
+
+	LOG_DBG("Queued TX %p", mem_block);
+
+	/* Check if interrupt wanted to get next TX buffer before current buffer
+	 * was queued. Do not move this check before queuing because doing so
+	 * opens the possibility for a race condition between this function and
+	 * data_handler() that is called in interrupt context.
+	 */
 	if (drv_data->state == I2S_STATE_RUNNING &&
 	    drv_data->next_tx_buffer_needed) {
-		nrfx_i2s_buffers_t next = { .p_tx_buffer = mem_block };
+		nrfx_i2s_buffers_t next = { 0 };
+
+		if (!get_next_tx_buffer(drv_data, &next)) {
+			/* Log error because this is definitely unexpected.
+			 * Do not return error because the caller is no longer
+			 * responsible for releasing the buffer.
+			 */
+			LOG_ERR("Cannot reacquire queued buffer");
+			return 0;
+		}
 
 		drv_data->next_tx_buffer_needed = false;
 
-		LOG_DBG("Next TX %p", mem_block);
+		LOG_DBG("Next TX %p", next.p_tx_buffer);
 
 		if (!supply_next_buffers(drv_data, &next)) {
 			return -EIO;
 		}
-	} else {
-		int ret = k_msgq_put(&drv_data->tx_queue,
-				     &mem_block,
-				     SYS_TIMEOUT_MS(drv_data->tx.cfg.timeout));
-		if (ret < 0) {
-			return ret;
-		}
 
-		LOG_DBG("Queued TX %p", mem_block);
 	}
 
 	return 0;