CRAS: alsa_jack - Simplify create node and associate jack

The caller to cras_jack_list_create() passes in a callback for
jack plugged state change event. And originally we override this
callback for node creation, which is unnecessary and causes
confusion to developers.

For example, it seems like jack plugging could add a new node
but that's actually not allowed and guarded by other implicit
assumptions.

This commit separates the tasks of creating node or associate
jack with existing node into another callback passed into
cras_alsa_jack_list_find_jacks_by_name_matching() so we have
a clear view of the functions' mission:

- During alsa iodev creation:
  - cras_jack_list_create(... jack_plug_cb)
  - cras_alsa_jack_list_find_jacks_by_name_matching(... node_cb)
  - node_cb is called on each jack found

- After iodev creation:
  - jack_plug_cb could be called to change node's "plugged" state
  - No chance for node_cb being called afterwards

This is a preliminary change for future refactor to CRAS ALSA
related modules.

Various unittest cases are fixed for the incorrect sequence run
around alsa_io and its node/jack creation.

BUG=None
TEST=unittest

Change-Id: I4fe5a873314617a7569bfcf17b958a29750bf945
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/5016543
Reviewed-by: Ching Yun Chang <whalechang@google.com>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>
Tested-by: chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com <chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com>

cras/src/server/cras_alsa_io.c

@@ -1417,39 +1417,15 @@ static void jack_output_plug_event(const struct cras_alsa_jack* jack,
   aio = (struct alsa_io*)arg;
   node = get_output_node_from_jack(aio, jack);
   jack_name = cras_alsa_jack_get_name(jack);
-  if (!jack_name || !strcmp(jack_name, "Speaker Phantom Jack")) {
-    jack_name = INTERNAL_SPEAKER;
-  }
 
-  // If there isn't a node for this jack, create one.
   if (node == NULL) {
-    if (aio->common.fully_specified) {
-      // When fully specified, can't have new nodes.
-      syslog(LOG_ERR, "No matching output node for jack %s!", jack_name);
-      return;
-    }
-    node = new_output(aio, NULL, jack_name);
-    if (node == NULL) {
-      return;
-    }
-
-    cras_alsa_jack_update_node_type(jack, &(node->base.type));
+    syslog(LOG_ERR, "No matching node found for jack %s", jack_name);
+    return;
   }
-
-  if (!node->jack) {
-    if (aio->common.fully_specified) {
-      syslog(LOG_ERR,
-             "Jack '%s' was found to match output node '%s'."
-             " Please fix your UCM configuration to match.",
-             jack_name, node->base.ucm_name);
-    }
-
-    // If we already have the node, associate with the jack.
-    node->jack = jack;
-    if (node->volume_curve == NULL) {
-      node->volume_curve =
-          create_volume_curve_for_jack(aio->common.config, jack);
-    }
+  if (node->jack == NULL) {
+    syslog(LOG_ERR, "Jack %s isn't associated with the matched node",
+           jack_name);
+    return;
   }
 
   syslog(LOG_DEBUG, "%s plugged: %d, %s", jack_name, plugged,
@@ -1487,7 +1463,6 @@ static void jack_input_plug_event(const struct cras_alsa_jack* jack,
                                   void* arg) {
   struct alsa_io* aio;
   struct alsa_input_node* node;
-  struct mixer_control* cras_input;
   const char* jack_name;
 
   if (arg == NULL) {
@@ -1497,34 +1472,19 @@ static void jack_input_plug_event(const struct cras_alsa_jack* jack,
   node = get_input_node_from_jack(aio, jack);
   jack_name = cras_alsa_jack_get_name(jack);
 
-  // If there isn't a node for this jack, create one.
   if (node == NULL) {
-    if (aio->common.fully_specified) {
-      // When fully specified, can't have new nodes.
-      syslog(LOG_ERR, "No matching input node for jack %s!", jack_name);
-      return;
-    }
-    cras_input = cras_alsa_jack_get_mixer_input(jack);
-    node = new_input(aio, cras_input, jack_name);
-    if (node == NULL) {
-      return;
-    }
+    syslog(LOG_ERR, "No matching node found for jack %s", jack_name);
+    return;
+  }
+  if (node->jack == NULL) {
+    syslog(LOG_ERR, "Jack %s isn't associated with the matched node",
+           jack_name);
+    return;
   }
 
   syslog(LOG_DEBUG, "%s plugged: %d, %s", jack_name, plugged,
          cras_alsa_mixer_get_control_name(node->mixer_input));
 
-  // If we already have the node, associate with the jack.
-  if (!node->jack) {
-    if (aio->common.fully_specified) {
-      syslog(LOG_ERR,
-             "Jack '%s' was found to match input node '%s'."
-             " Please fix your UCM configuration to match.",
-             jack_name, node->base.ucm_name);
-    }
-    node->jack = jack;
-  }
-
   cras_iodev_set_node_plugged(&node->base, plugged);
 
   check_auto_unplug_input_node(aio, &node->base, plugged);
@@ -2336,6 +2296,92 @@ struct cras_iodev* alsa_iodev_create(
   return NULL;
 }
 
+static void add_input_node_or_associate_jack(const struct cras_alsa_jack* jack,
+                                             void* arg) {
+  struct alsa_io* aio;
+  struct alsa_input_node* node;
+  struct mixer_control* cras_input;
+  const char* jack_name;
+
+  CRAS_CHECK(arg);
+
+  aio = (struct alsa_io*)arg;
+  node = get_input_node_from_jack(aio, jack);
+  jack_name = cras_alsa_jack_get_name(jack);
+
+  // If there isn't a node for this jack, create one.
+  if (node == NULL) {
+    if (aio->common.fully_specified) {
+      // When fully specified, can't have new nodes.
+      syslog(LOG_ERR, "No matching input node for jack %s!", jack_name);
+      return;
+    }
+    cras_input = cras_alsa_jack_get_mixer_input(jack);
+    node = new_input(aio, cras_input, jack_name);
+    if (node == NULL) {
+      return;
+    }
+  }
+
+  // If we already have the node, associate with the jack.
+  if (!node->jack) {
+    if (aio->common.fully_specified) {
+      syslog(LOG_ERR,
+             "Jack '%s' was found to match input node '%s'."
+             " Please fix your UCM configuration to match.",
+             jack_name, node->base.ucm_name);
+    }
+    node->jack = jack;
+  }
+}
+
+static void add_output_node_or_associate_jack(const struct cras_alsa_jack* jack,
+                                              void* arg) {
+  struct alsa_io* aio;
+  struct alsa_output_node* node;
+  const char* jack_name;
+
+  CRAS_CHECK(arg);
+
+  aio = (struct alsa_io*)arg;
+  node = get_output_node_from_jack(aio, jack);
+  jack_name = cras_alsa_jack_get_name(jack);
+  if (!jack_name || !strcmp(jack_name, "Speaker Phantom Jack")) {
+    jack_name = INTERNAL_SPEAKER;
+  }
+
+  // If there isn't a node for this jack, create one.
+  if (node == NULL) {
+    if (aio->common.fully_specified) {
+      // When fully specified, can't have new nodes.
+      syslog(LOG_ERR, "No matching output node for jack %s!", jack_name);
+      return;
+    }
+    node = new_output(aio, NULL, jack_name);
+    if (node == NULL) {
+      return;
+    }
+
+    cras_alsa_jack_update_node_type(jack, &(node->base.type));
+  }
+
+  if (!node->jack) {
+    if (aio->common.fully_specified) {
+      syslog(LOG_ERR,
+             "Jack '%s' was found to match output node '%s'."
+             " Please fix your UCM configuration to match.",
+             jack_name, node->base.ucm_name);
+    }
+
+    // If we already have the node, associate with the jack.
+    node->jack = jack;
+    if (node->volume_curve == NULL) {
+      node->volume_curve =
+          create_volume_curve_for_jack(aio->common.config, jack);
+    }
+  }
+}
+
 int alsa_iodev_legacy_complete_init(struct cras_iodev* iodev) {
   struct alsa_io* aio = (struct alsa_io*)iodev;
   const char* dev_name;
@@ -2362,7 +2408,11 @@ int alsa_iodev_legacy_complete_init(struct cras_iodev* iodev) {
     cras_alsa_mixer_list_inputs(mixer, new_input_by_mixer_control, aio);
   }
 
-  err = cras_alsa_jack_list_find_jacks_by_name_matching(aio->common.jack_list);
+  err = cras_alsa_jack_list_find_jacks_by_name_matching(
+      aio->common.jack_list,
+      iodev->direction == CRAS_STREAM_OUTPUT ? add_output_node_or_associate_jack
+                                             : add_input_node_or_associate_jack,
+      aio);
   if (err) {
     return err;
   }

cras/src/server/cras_alsa_jack.c

@@ -941,15 +941,26 @@ static int find_jack_controls(struct cras_alsa_jack_list* jack_list) {
  */
 
 int cras_alsa_jack_list_find_jacks_by_name_matching(
-    struct cras_alsa_jack_list* jack_list) {
+    struct cras_alsa_jack_list* jack_list,
+    jack_found_callback cb,
+    void* cb_data) {
+  struct cras_alsa_jack* jack;
   int rc;
 
   rc = find_jack_controls(jack_list);
   if (rc != 0) {
     return rc;
   }
 
-  return find_gpio_jacks(jack_list, NULL, NULL);
+  rc = find_gpio_jacks(jack_list, NULL, NULL);
+  if (rc != 0) {
+    return rc;
+  }
+
+  DL_FOREACH (jack_list->jacks, jack) {
+    cb(jack, cb_data);
+  }
+  return 0;
 }
 
 static int find_hctl_jack_for_section(struct cras_alsa_jack_list* jack_list,

cras/src/server/cras_alsa_jack.h

@@ -31,6 +31,16 @@ typedef void(jack_state_change_callback)(const struct cras_alsa_jack* jack,
                                          int plugged,
                                          void* data);
 
+/* Callback type for users of find_jacks_by_name_matching to define. It will
+ * be called when a jack is officially found.
+ * Args:
+ *     jack - The jack that is found.
+ *     data - Uesr defined pointer passed to
+ *            cras_alsa_jack_list_find_jacks_by_name_matching
+ */
+typedef void(jack_found_callback)(const struct cras_alsa_jack* jack,
+                                  void* data);
+
 /* Creates a jack list. The jacks can be added later by name matching or
  * fully specified UCM.
  * Args:
@@ -70,7 +80,9 @@ struct cras_alsa_jack_list* cras_alsa_jack_list_create(
  *   0 on success. Error code if there is a failure.
  */
 int cras_alsa_jack_list_find_jacks_by_name_matching(
-    struct cras_alsa_jack_list* jack_list);
+    struct cras_alsa_jack_list* jack_list,
+    jack_found_callback cb,
+    void* cb_data);
 
 /* Add the jack defined by the UCM section information.
  * Args: