mbox: Enforce callback registration before enabling the channel

Specify in the API that the callback must be registered before the
channel is enabled, fix the NRFX IPC driver to be compliant and change
the MBOX sample.

Signed-off-by: Carlo Caione <[email protected]>

drivers/mbox/mbox_nrfx_ipc.c

@@ -89,7 +89,7 @@ static int mbox_nrf_register_callback(const struct device *dev, uint32_t channel
 {
 	struct mbox_nrf_data *data = dev->data;
 
-	if (!is_rx_channel_valid(dev, channel)) {
+	if (channel >= IPC_CONF_NUM) {
 		return -EINVAL;
 	}
 
@@ -123,6 +123,10 @@ static int mbox_nrf_set_enabled(const struct device *dev, uint32_t channel, bool
 		return -EALREADY;
 	}
 
+	if (enable && (data->cb[channel] == NULL)) {
+		LOG_WRN("Enabling channel without a registered callback\n");
+	}
+
 	if (enable && data->enabled_mask == 0) {
 		irq_enable(DT_INST_IRQN(0));
 	}

include/drivers/mbox.h

@@ -405,7 +405,21 @@ static inline int z_impl_mbox_mtu_get(const struct device *dev)
 }
 
 /**
- * @brief Enable interrupts and callbacks for inbound channels.
+ * @brief Enable (disable) interrupts and callbacks for inbound channels.
+ *
+ * Enable interrupt for the channel when the parameter 'enable' is set to true.
+ * Disable it otherwise.
+ *
+ * Immediately after calling this function with 'enable' set to true, the
+ * channel is considered enabled and ready to receive signal and messages (even
+ * already pending), so the user must take care of installing a proper callback
+ * (if needed) using @a mbox_register_callback() on the channel before enabling
+ * it. For this reason it is recommended that all the channels are disabled at
+ * probe time.
+ *
+ * Enabling a channel for which there is no installed callback is considered
+ * undefined behavior (in general the driver must take care of gracefully
+ * handling spurious interrupts with no installed callback).
  *
  * @param channel Channel instance pointer.
  * @param enable Set to 0 to disable and to nonzero to enable.

samples/drivers/mbox/remote/src/main.c

@@ -31,13 +31,16 @@ void main(void)
 	mbox_init_channel(&tx_channel, dev, TX_ID);
 	mbox_init_channel(&rx_channel, dev, RX_ID);
 
+	if (mbox_register_callback(&rx_channel, callback, NULL)) {
+		printk("mbox_register_callback() error\n");
+		return;
+	}
+
 	if (mbox_set_enabled(&rx_channel, 1)) {
 		printk("mbox_set_enable() error\n");
 		return;
 	}
 
-	mbox_register_callback(&rx_channel, callback, NULL);
-
 	while (1) {
 
 		printk("Ping (on channel %d)\n", tx_channel.id);

samples/drivers/mbox/src/main.c

@@ -31,13 +31,16 @@ void main(void)
 	mbox_init_channel(&tx_channel, dev, TX_ID);
 	mbox_init_channel(&rx_channel, dev, RX_ID);
 
+	if (mbox_register_callback(&rx_channel, callback, NULL)) {
+		printk("mbox_register_callback() error\n");
+		return;
+	}
+
 	if (mbox_set_enabled(&rx_channel, 1)) {
 		printk("mbox_set_enable() error\n");
 		return;
 	}
 
-	mbox_register_callback(&rx_channel, callback, NULL);
-
 	while (1) {
 		k_sleep(K_MSEC(2000));