[bug report] ALSA: hda: cs35l56: Add support for speaker id

[bug report] ALSA: hda: cs35l56: Add support for speaker id

[Dan Carpenter]

[ It's weird that this is showing up now... - dan ]

Hello Richard Fitzgerald,

Commit 6f03b446cbae ("ALSA: hda: cs35l56: Add support for speaker
id") from Sep 18, 2023 (linux-next), leads to the following Smatch
static checker warning:

	sound/hda/codecs/side-codecs/cs35l56_hda.c:1084 cs35l56_hda_read_acpi()
	error: uninitialized symbol 'nval'.

sound/hda/codecs/side-codecs/cs35l56_hda.c
    1013 static int cs35l56_hda_read_acpi(struct cs35l56_hda *cs35l56, int hid, int id)
    1014 {
    1015         u32 values[HDA_MAX_COMPONENTS];
    1016         char hid_string[8];
    1017         struct acpi_device *adev;
    1018         const char *property, *sub;
    1019         size_t nval;
    1020         int i, ret;
    1021 
    1022         /*
    1023          * ACPI_COMPANION isn't available when this driver was instantiated by
    1024          * the serial-multi-instantiate driver, so lookup the node by HID
    1025          */
    1026         if (!ACPI_COMPANION(cs35l56->base.dev)) {
    1027                 snprintf(hid_string, sizeof(hid_string), "CSC%04X", hid);
    1028                 adev = acpi_dev_get_first_match_dev(hid_string, NULL, -1);
    1029                 if (!adev) {
    1030                         dev_err(cs35l56->base.dev, "Failed to find an ACPI device for %s\n",
    1031                                 dev_name(cs35l56->base.dev));
    1032                         return -ENODEV;
    1033                 }
    1034                 ACPI_COMPANION_SET(cs35l56->base.dev, adev);
    1035         }
    1036 
    1037         /* Initialize things that could be overwritten by a fixup */
    1038         cs35l56->index = -1;

We set cs35l56->index = -1;

    1039 
    1040         sub = acpi_get_subsystem_id(ACPI_HANDLE(cs35l56->base.dev));
    1041         ret = cs35l56_hda_apply_platform_fixups(cs35l56, sub, &id);
    1042         if (ret)
    1043                 return ret;
    1044 
    1045         if (cs35l56->index == -1) {

nval is uninitialized if cs35l56->index this condition is false.

    1046                 property = "cirrus,dev-index";
    1047                 ret = device_property_count_u32(cs35l56->base.dev, property);
    1048                 if (ret <= 0)
    1049                         goto err;
    1050 
    1051                 if (ret > ARRAY_SIZE(values)) {
    1052                         ret = -EINVAL;
    1053                         goto err;
    1054                 }
    1055                 nval = ret;
    1056 
    1057                 ret = device_property_read_u32_array(cs35l56->base.dev, property, values, nval);
    1058                 if (ret)
    1059                         goto err;
    1060 
    1061                 for (i = 0; i < nval; i++) {
    1062                         if (values[i] == id) {
    1063                                 cs35l56->index = i;
    1064                                 break;
    1065                         }
    1066                 }
    1067 
    1068                 /*
    1069                  * It's not an error for the ID to be missing: for I2C there can be
    1070                  * an alias address that is not a real device. So reject silently.
    1071                  */
    1072                 if (cs35l56->index == -1) {
    1073                         dev_dbg(cs35l56->base.dev, "No index found in %s\n", property);
    1074                         ret = -ENODEV;
    1075                         goto err;
    1076                 }
    1077         }
    1078 
    1079         if (IS_ERR(sub)) {
    1080                 dev_info(cs35l56->base.dev,
    1081                          "Read ACPI _SUB failed(%ld): fallback to generic firmware\n",
    1082                          PTR_ERR(sub));
    1083         } else {
--> 1084                 ret = cirrus_scodec_get_speaker_id(cs35l56->base.dev, cs35l56->index, nval, -1);
                                                                                               ^^^^
Warning here.

    1085                 if (ret == -ENOENT) {
    1086                         cs35l56->system_name = sub;
    1087                 } else if (ret >= 0) {
    1088                         cs35l56->system_name = kasprintf(GFP_KERNEL, "%s-spkid%d", sub, ret);
    1089                         kfree(sub);
    1090                         if (!cs35l56->system_name)
    1091                                 return -ENOMEM;
    1092                 } else {
    1093                         return ret;
    1094                 }
    1095         }
    1096 
    1097         cs35l56->base.reset_gpio = devm_gpiod_get_index_optional(cs35l56->base.dev,
    1098                                                                  "reset",
    1099                                                                  cs35l56->index,
    1100                                                                  GPIOD_OUT_LOW);
    1101         if (IS_ERR(cs35l56->base.reset_gpio)) {
    1102                 ret = PTR_ERR(cs35l56->base.reset_gpio);
    1103 
    1104                 /*
    1105                  * If RESET is shared the first amp to probe will grab the reset
    1106                  * line and reset all the amps
    1107                  */
    1108                 if (ret != -EBUSY)
    1109                         return dev_err_probe(cs35l56->base.dev, ret, "Failed to get reset GPIO\n");
    1110 
    1111                 dev_info(cs35l56->base.dev, "Reset GPIO busy, assume shared reset\n");
    1112                 cs35l56->base.reset_gpio = NULL;
    1113         }
    1114 
    1115         return 0;
    1116 
    1117 err:
    1118         if (ret != -ENODEV)
    1119                 dev_err(cs35l56->base.dev, "Failed property %s: %d\n", property, ret);
    1120 
    1121         return ret;
    1122 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter